-
Notifications
You must be signed in to change notification settings - Fork 570
[email protected] #6198
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
base: main
Are you sure you want to change the base?
[email protected] #6198
Conversation
Hello @kilograham, @tpudlik, modules you maintain (pico-sdk) have been updated in this PR. |
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.
Code Review
This pull request adds a pre-release version 2.2.1-develop.bcr.20250915-8fcd44a1
of the pico-sdk
module. The changes are well-structured and largely adhere to the Bazel Central Registry guidelines. I have one suggestion regarding the project's MODULE.bazel
file to ensure full compliance with the repository's style guide.
module( | ||
name = "pico-sdk", | ||
version = "2.2.1-develop.bcr.20250915-8fcd44a1", | ||
) |
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.
According to the BCR style guide, it is strongly suggested to specify bazel_compatibility
when using overlays.1 This module utilizes overlays through the build_file
attribute in several repository rules.
module( | |
name = "pico-sdk", | |
version = "2.2.1-develop.bcr.20250915-8fcd44a1", | |
) | |
module( | |
name = "pico-sdk", | |
version = "2.2.1-develop.bcr.20250915-8fcd44a1", | |
bazel_compatibility = [">=7.2.1"], | |
) |
Style Guide References
Footnotes
-
If using overlays, it's strongly suggested to include
bazel_compatibility
>= 7.2.1. ↩
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.
All modules in this PR have been approved by their maintainers. This PR will be merged if all presubmit checks pass.
Require module maintainers' approval for newly pushed changes.
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.
All modules in this PR have been approved by their maintainers. This PR will be merged if all presubmit checks pass.
Fix for presubmits in #6199 |
Require module maintainers' approval for newly pushed changes.
Hello @davexroth, @kilograham, @tpudlik, modules you maintain (pico-sdk) have been updated in this PR. |
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.
All modules in this PR have been approved by their maintainers. This PR will be merged if all presubmit checks pass.
Require module maintainers' approval for newly pushed changes.
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.
All modules in this PR have been approved by their maintainers. This PR will be merged if all presubmit checks pass.
Adds a pre-release of the pico-sdk that includes fixes for toolchain builtin collisions.