Conversation
|
Hello @keith, modules you maintain (opentelemetry-cpp) have been updated in this PR. |
![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 aims to expand the presubmit testing matrix for opentelemetry-cpp@1.24.0 to include Bazel versions 8.x and 9.x. While this is a valuable improvement for ensuring forward compatibility, the current implementation modifies an existing, published version. This violates the BCR's fundamental 'add-only' policy. My review includes a critical comment explaining that such changes require creating a new BCR patch version (e.g., 1.24.0.bcr.1) to maintain the immutability of published artifacts.
aaylward
force-pushed
the
otel_cpp_bazel_8_9
branch
from
0d095ba to
df000f9
Compare
aaylward
changed the title
aaylward
force-pushed
the
otel_cpp_bazel_8_9
branch
2 times, most recently
from
3ad68bd to
7c0f6a4
Compare
aaylward
changed the title
|
can you submit this PR upstream for feedback there? |
|
@keith it sounds like opentelemetry-cpp will merge the patch sometime soon. Are you ok with releasing a 1.24.0.bcr.1? or would you prefer to wait for 1.24.1 or 1.25.0? |
aaylward
force-pushed
the
otel_cpp_bazel_8_9
branch
2 times, most recently
from
ce70a6a to
c60466a
Compare
|
nice, i think we might as well merge |
macOS doesn't support dynamic_mode these days, everything ends up being static |
|
looks like the rules_foreign_cc version (idr why it depends on this) needs an update |
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.
Head branch was pushed to by a user without write access
aaylward
force-pushed
the
otel_cpp_bazel_8_9
branch
from
c60466a to
9ab2d87
Compare
bazel-io
dismissed stale reviews from keith and themself
Require module maintainers' approval for newly pushed changes.
|
@keith I added latest rules_foreign_cc -- thanks for the catch |
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.
Head branch was pushed to by a user without write access
aaylward
force-pushed
the
otel_cpp_bazel_8_9
branch
from
9ab2d87 to
2af38b5
Compare
bazel-io
dismissed stale reviews from keith and themself
Require module maintainers' approval for newly pushed changes.
|
CI says you have to rebase 🤔 |
- build with bazel 7, 8, and 9 - add linkstatic = True to the http_client_curl cc_library target
aaylward
force-pushed
the
otel_cpp_bazel_8_9
branch
from
2af38b5 to
2cf9802
Compare
|
tough, but fair |
3 participants
What's this?
I had to add this patch to get my project building with bazel 9 on linux. It worked fine on macos without the patch 🤷
What's changed?