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.

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

Auto-generate go_repository rules. #957

Closed
wants to merge 1 commit into from

Conversation

drigz
Copy link
Contributor

@drigz drigz commented Jun 18, 2019

The rules are autogenerated with:

bazel run //:gazelle -- update-repos -from_file=go.mod

And I manually rearranged and added the comment. This is quite messy, maybe @achew22 has an idea how it could be improved.

@johanbrandhorst johanbrandhorst self-assigned this Jun 18, 2019
@johanbrandhorst johanbrandhorst requested a review from achew22 June 18, 2019 15:06
@johanbrandhorst
Copy link
Collaborator

Will the existing CI tests pick up if this gets out of sync with go.mod?

@codecov-io
Copy link

Codecov Report

Merging #957 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #957   +/-   ##
=======================================
  Coverage   53.23%   53.23%           
=======================================
  Files          40       40           
  Lines        3999     3999           
=======================================
  Hits         2129     2129           
  Misses       1672     1672           
  Partials      198      198

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 883b764...fdbe471. Read the comment docs.

@drigz
Copy link
Contributor Author

drigz commented Jun 18, 2019

Will the existing CI tests pick up if this gets out of sync with go.mod?

No. update-repos doesn't have a diff mode, but we could try home-brewing something similar?

@johanbrandhorst
Copy link
Collaborator

Will the existing CI tests pick up if this gets out of sync with go.mod?

No. update-repos doesn't have a diff mode, but we could try home-brewing something similar?

How about rerunning the generation script in CI and git diffing? Not sure we care about keeping the comments. If there's a better way just do that but I think it'd be nice to ensure it's kept in sync.

@achew22
Copy link
Collaborator

achew22 commented Jun 19, 2019

update-repos has a great flag that can be set by the WORKSPACE directive

# gazelle:repository_macro dependencies%grpc_gateway_dependencies

then we could centralize all that logic into a file that is basically the same as a .sum file. Unfortunately this feature is not yet in a release of Gazelle (there hasn't been one since Feb and the feature went in here) so we can't depend on it without pointing at a specific commit. @jayconrod, do you have any ideas on how to move forward here? Is it safe to depend on a specific commit of Gazelle or would you prefer we wait till the next release?

@jayconrod
Copy link

Sorry for not responding earlier. If you want to depend on a specific commit, 2cf99d7 is probably best (or master after bazel-contrib/bazel-gazelle#555 is fixed).

I've been holding the release until Go module support is more fleshed out. bazel-contrib/bazel-gazelle#477 is the last important thing, but most people won't need that. It should be done soon.

@achew22
Copy link
Collaborator

achew22 commented Jun 23, 2019

@jayconrod, looks like bazel-contrib/bazel-gazelle@72ba271 is the commit to pin to, is that correct?

@jayconrod
Copy link

@achew22 Yep, that fixes the issue. Let me know if you run into any problems.

@johanbrandhorst
Copy link
Collaborator

What is the status of this? @drigz @achew22

@achew22
Copy link
Collaborator

achew22 commented Aug 5, 2019

I did it in d63917f. @drigz, thanks for the contribution!

@achew22 achew22 closed this Aug 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants