Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

lint has duplicate files when invoking union rules #19634

Closed
thejcannon opened this issue Aug 21, 2023 · 1 comment · Fixed by #19796
Closed

lint has duplicate files when invoking union rules #19634

thejcannon opened this issue Aug 21, 2023 · 1 comment · Fixed by #19796
Assignees
@thejcannon
Labels
bug

Comments

@thejcannon
Copy link
Member

Describe the bug
Split from #17403 (comment) the fmt/fix batch is built per-address, but fmt/fix are applied per-file. Since one file can represent multiple addresses, we should de-dup.
@thejcannon thejcannon added the bug label Aug 21, 2023
@thejcannon thejcannon self-assigned this Aug 21, 2023
@thejcannon thejcannon changed the title fmt/fix has duplicate files when invoking union rules lint has duplicate files when invoking union rules Sep 7, 2023
@thejcannon
Copy link
Member Author

Editing, lint is the one with duplicates

@thejcannon thejcannon mentioned this issue Sep 7, 2023
Merged
thejcannon added a commit that referenced this issue Sep 15, 2023
@thejcannon
Tweak fix partitioning to closer align to lint partitioning (#19796)
4472383 
#17403 (comment)
describes a bit of a split between `lint` and `fix` in terms of
batching.

This change fixes #19634 by switching `AbstractFixRequest.files` to
report `self.snapshot.files` which is already de-duplicated. (Note that
the elements in the case of `lint` were the filepaths directly
translated from addresses. Maybe it's worth de-duplicating at a higher
layer, but 🤷‍♂️ )

This change also fixes #17403 (which really is a duplicate of #15069
manifest in new and exciting ways) by de-duplicating the `fix`-specific
batching strategy. It didn't sit right to me that in the reproduction
repo we did 1 batches of 2 files for `fix`, but 1 batch of 4 files for
`lint`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@thejcannon thejcannon

Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Tweak fix partitioning to closer align to lint partitioning pantsbuild/pants
1 participant
@thejcannon