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

refactor: use buildeventstream alias for clarity #70

Merged
merged 1 commit into from
Nov 18, 2021

Conversation

f0rmiga
Copy link
Contributor

@f0rmiga f0rmiga commented Nov 18, 2021

Since the buildeventstream proto is vendored and lives under third-party, we do some workarounds to make it appear to come from //bazel/buildeventstream/proto.

We enforce it with Gazelle now. Makes it simpler to reason about when importing aspect.build/cli/bazel/buildeventstream/proto.

Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com>

alias(
name = "proto",
actual = "//third-party/github.com/bazelbuild/bazel/src/main/java/com/google/devtools/build/lib/buildeventstream/proto",
Copy link
Member

Choose a reason for hiding this comment

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

nit: I think it's also fine for us to vendor the file itself here, the third-party directory structure isn't mandated anywhere

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I believe there's no obligation to do the way I did. Still, I like to keep it there as it makes it clear the file is vendored and not authored by us.

@f0rmiga f0rmiga merged commit aa86b89 into main Nov 18, 2021
@f0rmiga f0rmiga deleted the use-buildevents-alias branch November 18, 2021 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants