-
Notifications
You must be signed in to change notification settings - Fork 86
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 dev QA script #6092
Fix dev QA script #6092
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @maggie-lou and the rest of your teammates on Graphite |
460f9f9
to
d19f17a
Compare
}, | ||
{ | ||
"name": "bazel", |
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.
nit: instead of removing bazel
we could also pin at a known good commit and bazel version. I think in general we probably want to pin all of these to a known good commit (plus a bazel version if a .bazelversion
is missing from the repo), so that if these repos break then we don't have to block the release
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.
Done - pinned all the commit shas to current HEAD. For the bazel repo, pinned to the parent commit before they cleared their WORKSPACE file. Also pinned bazel version to 7.0.2
Was going back and forth on whether we should pin the bazel version, because we might want to know if builds are breaking on a later bazel version. But I think pinning it will make things more stable, and we can find that out other ways
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.
I'm actually having trouble getting bazel
repo to build on an older commit. I want to make sure these fixes get in before the next release, so just going to remove bazel for now. We'll still have a go, c++, and python test build
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.
LGTM (we can maybe pin these to known good commits another time, since that seems like it might be slightly tricky when using the github checkout action)
af799b7
to
6dc8102
Compare
817688c
to
5cb4da2
Compare
bazel
repo build because it's been fully migrated to bzlmod, and we haven't done enough testing to guarantee our toolchain work well with bzlmod)Related issues: Fixes https://github.com/buildbuddy-io/buildbuddy-internal/issues/3167