bazel-gazelle
bazel-gazelle copied to clipboard
bazelbuild
Move most of gazelle main package into a subpackage
What type of PR is this?
Feature
What package or component does this PR mostly affect?
cmd/gazelle
What does this PR do? Why is it needed?
For people who are extending gazelle with other languages, IDE development is a chore. There's no easy way to produce a working gazelle binary with a desired set of languages configured. Allowing the implementation of /cmd/gazelle to be accessible enables IDE development and debug without needing bazel code generation.
Which issues(s) does this PR fix?
Fixes #1291
@achew22 @fmeum could you guys take a look? This and https://github.com/bazelbuild/bazel-gazelle/pull/1303 both work toward making it easier to build a Gazelle extension.
I think we should be very very careful about extending our public interface. It sounds to me like the intent here is to only expose Run and nothing else. Can we hide GetDefaultWorkspaceDirectory? Are there any others that are now exported that weren't before?
I think we should be very very careful about extending our public interface. It sounds to me like the intent here is to only expose
Runand nothing else. Can we hideGetDefaultWorkspaceDirectory? Are there any others that are now exported that weren't before?
It's just those two. Funny enough, @sluongng and I went back and forth a bit on exactly how that should be formulated.
Annnd.... now this PR is stale, since it's a month old. I'd love to update this, but can we first agree on whether or not this can be landed? Or what changes are needed to land it?
I'd like to see development on this PR resume. Maintaining a separate copy of cmd/gazelle in https://github.com/stackb/rules_proto/tree/master/cmd/gazelle is a pain.
@achew22 I think this PR is currently pending your review? Do you have any objection merging this as-is or suggestion on how to improve this?
We also have to patch Gazelle to make things work on our end, some having this PR merged would be very nice.
I suspect the age of the PR will require someone making sure all the changes that have occurred in cmd/gazelle since July 2022 are faithfully propagated to this one. I'd be hesitant to merge it as-is without careful review of that even if checks are passing.
Indeed, this branch is full of conflicts. I'll basically need to re-do this PR. But I also don't want to waste time if this can't land.
Basically, I'm waiting on someone with the power to merge PRs to actively engage on this and offer a path to getting this merged.
FWIW I'm +1 on accepting this. @achew22 I believe your question was answered previously, do you have any remaining concerns?
My concerns mostly revolve around that this isn't really an interface ever designed to be part of a v1 API. As evidenced by the buzz around this PR, I'm inclined to believe that if we expose this API, people will come to depend on it in it's current form. I would love to see a refactor of this logic to allow the runner to be embedded into other applications, but I would like API design to be something that is at least considered instead of "just use the old API because it is there".
If it makes a difference, this repo being on v0.31.0 (rather than say, 1.0) means that there's no guarantee of API compatibility, at least in the Go world. I personally just want to be able to use this repo as a Go module, knowing that when I update to a newer gazelle it might break my client code that uses it.