-
Notifications
You must be signed in to change notification settings - Fork 90
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
[Bazel6] Upgrade to Bazel 6.0.0 #616
[Bazel6] Upgrade to Bazel 6.0.0 #616
Conversation
@mattrobmattrob awesome stuff 👏 ! |
7e088ec
to
0c31bf3
Compare
0c31bf3
to
9259101
Compare
9259101
to
38d2ed2
Compare
After this change, CI will only validate against Bazel 6, right? If we're actively supporting Bazel 5 & Bazel 6, should we add a fork of the test configs that continues to run against Bazel 5? |
For sure, @justinseanmartin. I wanted to have a conversation with you all to determine what we want to do. I'm okay not merging this until we feel like "we support 6.0.0 and newer" is how we feel. I'm hesitant to do much more in CI given those builds are already pretty slow/we're dealing with PR machine contention as it goes. But if you all are cool with it (you all are likely the majority of PR load at any time) then I'm happy to create a Bazel 6 test replication once it's officially released. |
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.
This seems to pass basic smoke tests on our end BTW ❤️ . I think it's reasonable to bump this onto 6.0 on CI given the worrysome 6.x.x changes are backwards compatible to a 5.x.x release. Someone will have to vet it more in-depth I think
The 6.0 release seems to be delayed but is coming soon - does it seem reasonable to merge it when we can get .bazelversion
onto 6.0.0
?
Thanks for testing it, @jerrymarino! My preference is to move |
SGTM overall! |
38d2ed2
to
29dfe7b
Compare
29dfe7b
to
8988326
Compare
@mattrobmattrob this looks awesome - and my understanding is we should be ready to land it. Should we cut over to it 6.0 to celebrate the release drop that just happened? |
I already did, @jerrymarino 😊 |
That's great to hear - thanks again for making this happen! 👏 Should we land this bump of the version/project transitions then? |
I'm good with it if you all are. 👍 |
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.
🚀
Yeah for sure 🚢 🎄! |
Changes
--experimental_worker_allow_json_protocol
([Bazel6] Remove--experimental_worker_allow_json_protocol
#615)ObjcProvider.direct_headers
([Bazel6] Handle removal ofObjcProvider.direct_headers
#620)CcInfo
forprecompiled_apple_resource_bundle
([Bazel6] AddCcInfo
toprecompiled_apple_resource_bundle
#622)_framework_packaging
([Bazel6] Avoid copying in.h
directories intoFoo.framework/Headers
#624)header
not recognized key inObjcProvider
([Bazel6] RemoveObjcProvider.module_map
->ObjcProvider.header
#625)