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

Fix Bazel build and add a GitHub action to exercise the build on different platforms #43

Merged
merged 8 commits into from
Aug 30, 2021

Conversation

kimurayu45z
Copy link
Contributor

This is an extended branch from https://github.com/KimuraYu45z/protobuf-rules-gen/tree/master
so plz see #42 at first.

In this branch, I wrote the github actions workflow to emit the built binary files across platform matrix.
This can help M1 mac users and so on.

Copy link

@var-const var-const left a comment

Choose a reason for hiding this comment

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

Thank you! Overall this looks great, just a couple of nitpicks.

.github/workflows/binary.yml Outdated Show resolved Hide resolved
bazel/repositories.bzl Outdated Show resolved Hide resolved
bazel/repositories.bzl Outdated Show resolved Hide resolved
bazel/repositories.bzl Show resolved Hide resolved
@kimurayu45z kimurayu45z requested a review from var-const August 30, 2021 13:11
@var-const var-const changed the title Actions Fix Bazel build and add a GitHub action to exercise the build on different platforms Aug 30, 2021
Copy link

@var-const var-const left a comment

Choose a reason for hiding this comment

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

Thank you for the fix!

My only remaining comment is to expand the PR name to be a little more descriptive. I went ahead and renamed it for faster turnaround -- please let me know if you'd prefer any changes (the PR name can be changed even when it's merged).

@var-const var-const merged commit 860e791 into FirebaseExtended:master Aug 30, 2021
This was referenced Aug 31, 2021
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