[email protected] #6366
Conversation
|
Hello @c8ef, modules you maintain (onetbb) have been updated in this PR. |
|
@bazel-io skip_check unstable_url |
bazel-io
added
the
skip-url-stability-check
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds version 2022.2.0 for the onetbb module. The changes are mostly correct and follow the add-only policy of the BCR. However, I've found a critical issue in the presubmit.yml file that will cause presubmit checks to fail. I've also included a high-severity suggestion for the BUILD.bazel overlay to better align with the BCR style guide, such as adding a target alias and documenting the overlay's creation. Please review the comments for details.
| build_targets: | ||
| - '@onetbb//:tbb' |
There was a problem hiding this comment.
There are two issues with the presubmit configuration:
- Critical: The
build_targetsentry'@onetbb//:tbb'is incorrect. For anonymous module tasks, targets must be referenced from the workspace root (e.g.,//:tbb). The current configuration will cause presubmit to fail. - High: The configuration only builds the library but doesn't run any of the tests defined in
BUILD.bazel. The BCR style guide strongly encourages running tests in presubmit to ensure correctness.1
The suggestion below fixes the target path and adds test_targets to run all available tests.
build_targets:
- '//:tbb'
test_targets:
- '//...'Style Guide References
Footnotes
-
The style guide recommends adding test targets to presubmit configurations to verify module correctness. ↩
bazel-io
left a comment
bazel-io
left a comment
There was a problem hiding this comment.
All modules in this PR have been approved by their maintainers. This PR will be merged if all presubmit checks pass.
|
Need a BCR maintainer review, since presubmit was changed |
Wyverald
added
the
presubmit-auto-run
Vertexwahn
force-pushed
the
[email protected]
branch
from
bef06c9 to
a8e01b3
Compare
|
Hello @c8ef, modules you maintain (onetbb) have been updated in this PR. |
bazel-io
dismissed stale reviews from c8ef and themself
Require module maintainers' approval for newly pushed changes.
bazel-io
left a comment
bazel-io
left a comment
There was a problem hiding this comment.
All modules in this PR have been approved by their maintainers. This PR will be merged if all presubmit checks pass.
Vertexwahn
force-pushed
the
[email protected]
branch
from
2730382 to
70a1d22
Compare
bazel-io
dismissed
their stale review
Require module maintainers' approval for newly pushed changes.
Vertexwahn
force-pushed
the
[email protected]
branch
from
70a1d22 to
738dc05
Compare
bazel-io
left a comment
bazel-io
left a comment
There was a problem hiding this comment.
All modules in this PR have been approved by their maintainers. This PR will be merged if all presubmit checks pass.
bazel-io
dismissed
their stale review
Require module maintainers' approval for newly pushed changes.
bazel-io
left a comment
bazel-io
left a comment
There was a problem hiding this comment.
All modules in this PR have been approved by their maintainers. This PR will be merged if all presubmit checks pass.
bazel-io
added
the
auto-merged
4 participants
[email protected]