close
Skip to content

ci: ignore PERF401 (manual list comprehension) across the repo#2201

Merged
tonyandrewmeyer merged 1 commit intocanonical:mainfrom
tonyandrewmeyer:exclude-perf401
Nov 28, 2025
Merged

ci: ignore PERF401 (manual list comprehension) across the repo#2201
tonyandrewmeyer merged 1 commit intocanonical:mainfrom
tonyandrewmeyer:exclude-perf401

Conversation

@tonyandrewmeyer
Copy link
Copy Markdown
Collaborator

@tonyandrewmeyer tonyandrewmeyer commented Nov 28, 2025

Disables PERF401, which suggests using a list comprehension rather than for loops. We still want to do this in many cases, but would prefer to decided on a case-by-case basis, rather than have it enforced as a rule.

Examples of where this is suggested (all for using list.extend and all in testing, but that's what we currently have as exceptions):

testing/src/scenario/_consistency_checker.py:556:17: PERF401 Use `list.extend` to create a transformed list
    |
554 |           for relation in _get_relations(endpoint):
555 |               if not isinstance(relation, PeerRelation):
556 | /                 errors.append(
557 | |                     f'endpoint {endpoint} is a peer relation; '
558 | |                     f'expecting relation to be of type PeerRelation, got {type(relation)}',
559 | |                 )
    | |_________________^ PERF401
560 |
561 |       known_endpoints = [a[0] for a in all_relations_meta]
    |
    = help: Replace for loop with list.extend

testing/src/scenario/_consistency_checker.py:564:13: PERF401 Use `list.extend` to create a transformed list
    |
562 |     for relation in state.relations:
563 |         if (ep := relation.endpoint) not in known_endpoints:
564 |             errors.append(f'relation endpoint {ep} is not declared in metadata.')
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PERF401
565 |
566 |     seen_ids: set[int] = set()
    |
    = help: Replace for loop with list.extend

testing/src/scenario/_consistency_checker.py:714:21: PERF401 Use `list.extend` to create a transformed list
    |
712 |               for attr in ('level', 'startup', 'threshold'):
713 |                   if getattr(check, attr) != getattr(plan_check, attr):
714 | /                     errors.append(
715 | |                         f'container {container.name!r} has a check {check.name!r} with a '
716 | |                         f'different {attr!r} ({getattr(check, attr)}) '
717 | |                         f'than the plan ({getattr(plan_check, attr)}).',
718 | |                     )
    | |_____________________^ PERF401
719 |
720 |       return Results(errors, [])
    |
    = help: Replace for loop with list.extend

@tonyandrewmeyer tonyandrewmeyer changed the title ci: ignore perf401. ci: ignore PERF401 (manual list comprehension) across the repo Nov 28, 2025
@tonyandrewmeyer tonyandrewmeyer marked this pull request as ready for review November 28, 2025 01:42
Copilot AI review requested due to automatic review settings November 28, 2025 01:42
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR disables the PERF401 ruff rule, which flags manual list comprehension patterns and suggests using list.extend() or list comprehensions. The team prefers to handle these suggestions on a case-by-case basis rather than enforcing it as a blanket rule.

Key Changes

  • Added PERF401 to the ignore list in the ruff configuration

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Collaborator

@benhoyt benhoyt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, thanks. And I didn't see any existing uses using rg '\.extend\(' that we should undo.

@tonyandrewmeyer tonyandrewmeyer merged commit 5bbb685 into canonical:main Nov 28, 2025
64 checks passed
@tonyandrewmeyer tonyandrewmeyer deleted the exclude-perf401 branch November 28, 2025 03:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants