Prefer go_grpc_library #1711
Prefer go_grpc_library #1711
Conversation
mering
force-pushed
the
prefer-go_grpc_library
branch
3 times, most recently
from
549842d
to
d7eceaf
Compare
mering
force-pushed
the
prefer-go_grpc_library
branch
from
d7eceaf
to
10f805d
Compare
language/go/fix.go
Outdated
| func migrateGrpcCompilers(c *config.Config, f *rule.File) { | ||
| for _, r := range f.Rules { | ||
| if r.Kind() != "go_grpc_library" || r.ShouldKeep() || r.Attr("compilers") != nil { | ||
| if r.Kind() != "go_proto_library" || r.ShouldKeep() || r.Attr("compilers") == nil || len(r.AttrStrings("compilers")) != 1 || !strListAttrContains(r, "compilers", oldGrpcCompilerLabel) { |
There was a problem hiding this comment.
How can I check if the corresponding proto .hasServices() like in generate.go instead of checking for the old compiler name?
There was a problem hiding this comment.
I removed this fixer to unblock this PR as we do no longer need to convert in this direction. Existing rules will just be kept as is which should continue working. Adding a fixer to convert the opposite direction might be added in a future PR.
repository.md
Outdated
There was a problem hiding this comment.
I tried updating the docs via bazel run //docs:update but the tests keep failing on GitHub (the docs tests were successful on my local machine though).
There was a problem hiding this comment.
I can look into that when the other issues are resolved, don't worry.
|
Is there a way to configure the behavior of |
I don't know but I would avoid such things whenever possible as they just introduce a lot of additional complexity and locations one has to check when debugging. |
|
I would also avoid this. With Bzlmod, each version of Gazelle can declare which minimum version of rules_go it is compatible with. Without Bzlmod, version resolution is non-existent anyway, so I'm fine with "users have to update Gazelle and rules_go in lockstep" since that's the most promising approach to solve any kind of WORKSPACE issue anyway. |
|
I submitted bazel-contrib/rules_go#3818 to temporarily undo the deprecation until we figure out the Gazelle migration. |
This allows for easier compiler upgrades via rules_go upgrades
mering
force-pushed
the
prefer-go_grpc_library
branch
from
2e2a44c
to
8f3cf59
Compare
|
Tests are passing locally now. This should be enough to unblock this PR and adding back the deprecation in rules_go. As my company does not need any such Gazelle changes, unfortunately I can't spend more time on this. |
There was a problem hiding this comment.
I prefer generating go_proto_library and set compilers attributes, which is more flexible and more consistent with Gazelle's philosophy of "no hidden magic" (Yes, there are codebases not using Gazelle and extensively use macros to make magic happen, so people don't need to write too much build files). This is the direction we are moving to, unless there is something I am missing.
It's OK for Gazelle to generate the new v2 grpc plugin by default when # gazelle:go_grpc_compilers is not set to encourage new codebase to use the new plugin. But for existing codebase, they can decide their timeline of migration by setting # gazelle:go_grpc_compilers at different levels of directory.
What type of PR is this?
Bug fix
What package or component does this PR mostly affect?
language/go
What does this PR do? Why is it needed?
See issue.
Which issues(s) does this PR fix?
Fixes #1709
Other notes for review
Depends on bazel-contrib/rules_go#3812