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

Update Bazel version to 5.1.1 #110

Merged
merged 6 commits into from
May 10, 2022
Merged

Update Bazel version to 5.1.1 #110

merged 6 commits into from
May 10, 2022

Conversation

iverson52000
Copy link
Contributor

@iverson52000 iverson52000 commented May 4, 2022

Update Bazel version to 5.1.1 and fix breaking changes from transitive dependencies
image


This change is Reviewable

Copy link
Contributor

@stevenwarejones stevenwarejones left a 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: :shipit: complete! all files reviewed, all discussions resolved (waiting on @renjiezh, @SanjayVas, @tristanvuong2021, and @yunyeng)

Copy link
Contributor

@stevenwarejones stevenwarejones left a 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?

Copy link
Contributor

@renjiezh renjiezh left a 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)

Copy link
Contributor Author

@iverson52000 iverson52000 left a 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

Copy link
Contributor

@renjiezh renjiezh left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @SanjayVas, @stevenwarejones, @tristanvuong2021, and @yunyeng)

Copy link
Member

@SanjayVas SanjayVas left a 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

Copy link
Contributor

@stevenwarejones stevenwarejones left a 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)

@SanjayVas
Copy link
Member

I believe this effectively depends on #111 to make sure we don't break Spanner integration.

Copy link
Contributor Author

@iverson52000 iverson52000 left a 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)

Copy link
Contributor Author

@iverson52000 iverson52000 left a 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: :shipit: complete! all files reviewed, all discussions resolved (waiting on @tristanvuong2021 and @yunyeng)

@iverson52000 iverson52000 merged commit 9d6bf8a into main May 10, 2022
@iverson52000 iverson52000 deleted the alberthsuu-update-bazel branch May 10, 2022 18:03
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.

None yet

4 participants