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

add importpath_aliases to go_proto_library #2608

Merged
merged 3 commits into from
Aug 20, 2020

Conversation

tian000
Copy link
Contributor

@tian000 tian000 commented Aug 17, 2020

What type of PR is this?
Bug fix

What does this PR do? Why is it needed?
It allows go_proto_library to accept importpath_aliases as an argument. It is needed because gazelle generates go_proto_library rules that have importpath_aliases as an argument.

Which issues(s) does this PR fix?
Fixes #2358

@jayconrod
Copy link
Contributor

This PR seems incomplete? It doesn't actually implement importpath_aliases or test that it works.

@tian000
Copy link
Contributor Author

tian000 commented Aug 18, 2020

@jayconrod in Line 19 of tests/core/go_proto_library/protos_alias_test.go, I import the protos using the alias path, and then ensure that the generated types exist.

@tian000
Copy link
Contributor Author

tian000 commented Aug 18, 2020

Im pretty new to rules_go but i think it works out of the box because go_context grabs the attribute here:
https://github.com/ConsultingMD/rules_go/blob/3b5f0c950e0deec63cd77cdcec7765b5f41fefbd/go/private/context.bzl#L441

Copy link
Contributor

@jayconrod jayconrod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, I'm sorry, you're totally right. I forgot how this worked and was thinking that adding this attribute would be a lot more complicated.

This PR looks good, just have two small comments. Thanks for fixing this.

tests/core/go_proto_library/protos_alias_test.go Outdated Show resolved Hide resolved
tests/core/go_proto_library/BUILD.bazel Show resolved Hide resolved
@tian000 tian000 requested a review from jayconrod August 19, 2020 23:12
@tian000
Copy link
Contributor Author

tian000 commented Aug 19, 2020

np! @jayconrod responded to your comments!

Copy link
Contributor

@jayconrod jayconrod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@jayconrod jayconrod merged commit 631e26b into bazelbuild:master Aug 20, 2020
jayconrod pushed a commit to jayconrod/rules_go that referenced this pull request Aug 21, 2020
This reverts commit 631e26b.

rules_go CI started failing after bazelbuild#2608 was merged.

For bazelbuild/bazel#11885
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.

importpath_aliases not implemented for go_proto_library
3 participants