-
Notifications
You must be signed in to change notification settings - Fork 2
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
Update Bazel version to 5.1.1 #110
Conversation
…ccessfully" This reverts commit d4ad07e.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any potential downstream issues with panel-exchange-client or cross-media-measurement to adopt this?
Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @renjiezh, @SanjayVas, @tristanvuong2021, and @yunyeng)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @iverson52000, @renjiezh, @SanjayVas, @tristanvuong2021, and @yunyeng)
build/com_github_grpc_grpc/repo.bzl
line 26 at r1 (raw file):
name = "com_github_grpc_grpc", sha256 = "8eb9d86649c4d4a7df790226df28f081b97a62bf12c5c5fe9b5d31a29cd6541a", strip_prefix = "grpc-1.36.4",
is this not compatible with bazel 5?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hint: tuning here (https://github.com/world-federation-of-advertisers/cross-media-measurement/blob/main/build/repositories.bzl#L25-L30) you can check if cross-media-measurement works with the new common-jvm. You could specify the commit instead of version in wfa_repo_archive()
so that you test it without creating a new release.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @iverson52000, @SanjayVas, @tristanvuong2021, and @yunyeng)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stevenwarejones Yes panel-exchange-client, cross-media-measurement, and other downstream repos have to be updated too to build with Bazel 5
@renjiezh Yes I have tested all the downstream repos with this new common-jvm and have fixed the breaking changes. I am opening PRs starting from common-jvm and common-cpp
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @SanjayVas, @stevenwarejones, @tristanvuong2021, and @yunyeng)
build/com_github_grpc_grpc/repo.bzl
line 26 at r1 (raw file):
Previously, stevenwarejones (Steven Ware Jones) wrote…
is this not compatible with bazel 5?
No this has to be updated to newer version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @SanjayVas, @stevenwarejones, @tristanvuong2021, and @yunyeng)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @iverson52000, @stevenwarejones, @tristanvuong2021, and @yunyeng)
build/com_github_grpc_grpc/repo.bzl
line 26 at r1 (raw file):
name = "com_github_grpc_grpc", sha256 = "e18b16f7976aab9a36c14c38180f042bb0fd196b75c9fd6a20a2b5f934876ad6", strip_prefix = "grpc-1.45.2",
nit: 1.46.0 was released for both this and grpc-java
Code quote:
1.45.2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @iverson52000, @tristanvuong2021, and @yunyeng)
I believe this effectively depends on #111 to make sure we don't break Spanner integration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct. I will merge in #111 (Update google-cloud-spanner to 6.23.3.) when it's closed before merging this PR
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @iverson52000, @tristanvuong2021, and @yunyeng)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 4 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @tristanvuong2021 and @yunyeng)
Update Bazel version to 5.1.1 and fix breaking changes from transitive dependencies
This change is