feat(gazelle): simplify gazelle_python.yaml#2048
Conversation
050df26 to
3f00830
Compare
aignas
left a comment
There was a problem hiding this comment.
@linzhp, @dougthor42, you have bigger projects using gazelle so it would be great to hear if this has any negative impacts. @alexeagle, since you have clients using gazelle maybe you have time to test this change out with bigger projects to let us know how this would work in cases where there is a lot of traffic and changes to what internal files packages have would cause merge conflicts?
|
Just FYI this is still on my radar. I expect to have some time in the next 48hrs. |
|
Yeah, I'll wait to give enough time for people to look into this. |
|
Note: I had to cherry-pick cca1d4e (#2058) to get this to work, but that's expected.
We have 505 BUILD.bazel files. I tried randomly deleting some of them and having gazelle remake them. No change1 👍 $ find src/ -type f -name BUILD.bazel -print0 | xargs -0 -I {} bash -c '[ $[ $RANDOM % 100 ] == 0 ] && echo deleting {} && rm {}'
list of 6 files
$ bazel run //:gazelle
$ git status
On branch u/dthor/bazel-simplify-gazelle-check
nothing to commit, working tree cleanI did the "randomly delete" again a couple more times. All good! I also ran some Overall I think this is 💯 . Very nice! 🎉 🥳 Footnotes
|
|
Shorten gazelle_python.yaml greatly. I didn't run into issues with some quick test |
aignas
left a comment
There was a problem hiding this comment.
Minor nit, but otherwise looks good. I'll wait for a few more days for someone form Aspect to chime in.
gazelle/python/testdata/with_third_party_requirements_from_imports_legacy/gazelle_python.yaml
Outdated
Show resolved
Hide resolved
Signed-off-by: hunshcn <hunsh.cn@gmail.com>
|
I'm sorry, I have no time the rest of this month - maybe @jbedard has an idea since he worked on python gazelle recently |
…ap with a gazelle_python.yaml dep module (#2160) In #2048, `FindThirdPartyDependency` was updated to walk up the module import path to try to find a match in gazelle_python.yaml. This is unnecessary, as the main resolve loop will accomplish the same task. Additionally, the change broke the ability to configure a more specific resolve override or resolve more specific indexed libraries. For a real-world example of where this is a problem, `pytype` has a `third_party` module at its top-level. In a repo that also has a `third_party` directory, we can no longer resolve our indexed libraries in `third_party`. When the resolve loop tried to resolve `third_party.foo.bar`, `FindThirdPartyDependency` will immediately match `third_party` and not give the resolve loop a chance to look in the rule index for `third_party.foo.bar`. The same issue appears for providing overrides that are more specific (see the updated testcase). This PR reverts the change to FindThirdPartyDependency and updates the testcases to ensure that we can still resolve specific indexed packages, explicit resolve overrides, and third party modules even when there is an overlap in the top-level module name.
Simplify and make gazelle_python.yaml have only top level package name.
It would work well in cases to reduce merge conflicts.
Resolve #1890
This is a compatible change and seems to have no side effects, so no options are provided.