-
Notifications
You must be signed in to change notification settings - Fork 536
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.
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
fix(gazelle): delete invalid py_library and use correct NonEmptyAttrs for py_* #1887
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! Could you add a test that ensures that we don't regress in the future and adjust the changelog notes please?
changelog has been added. It requires two test part.
But I have no idea who to add test about 2. |
For 2. you should be able to construct a test similar to how we have them in https://github.com/bazelbuild/rules_python/tree/main/gazelle/python/testdata You can have the bazel file before running gazelle and after running gazelle, where the test framework will ensure that it works as you expect it to. |
test added @aignas |
031e34f
to
52b4bf5
Compare
ping @aignas |
could we continue? |
54fd2c4
to
76e5f52
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general LGTM, thank you very much for the PR.
Could you please modify the PR description so that is more useful as a git commit message when people look at the history later?
Something like:
Before gazelle would leave generated py_library targets aroundeven when no files in a directory exist because X. With this change gazelle correctly handles this case.
Summary:
- Change private attrs
- Something else.
fc5c57b
to
b666908
Compare
#1887 incorrectly generated an empty py_library for all dirs, although it will eventually be removed because it is empty, but it will easily conflict with other existing rules. Fix this error, and only generate an empty py_library for bazel packages with the same name py_library rule.
Before gazelle would leave generated py_library targets aroundeven when no files in a directory exist because X. With this change gazelle correctly handles this case.