update otel proto revision to b096b70b2ffe9beb65a716cf47d5e5db80a9e930#1607
Conversation
WalkthroughUpdated the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Cargo.toml (1)
100-106:⚠️ Potential issue | 🟡 MinorClarify the intent of the
opentelemetry-protoversion pinning.The commit
b096b70b2ffe9beb65a716cf47d5e5db80a9e930is valid and successfully resolved. However, the inline comment "add custom branch with fix until it gets merged" is misleading. This commit is actually an upstream version bump to0.10.0(opentelemetry-proto), not a custom fix.If this is a temporary workaround, clarify what fix is needed and when you expect to unpin it. If this is the intended version, update the comment to reflect that.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Cargo.toml` around lines 100 - 106, The comment about pinning opentelemetry-proto to rev b096b70b2ffe9beb65a716cf47d5e5db80a9e930 is misleading; update the Cargo.toml comment near the opentelemetry-proto dependency to state whether this rev is a temporary workaround (describe the missing fix and expected unpin condition/timeframe) or the intended upstream bump to 0.10.0 (state that it tracks the upstream version bump), so future readers know why the git rev is used for opentelemetry-proto and when it can be removed or converted to a crate version.
🧹 Nitpick comments (1)
Cargo.toml (1)
91-99: Document the reason for using a git dependency.The comment "add custom branch with fix until it gets merged" is vague. Consider documenting:
- What specific fix or feature this commit provides
- Link to the upstream issue or PR being tracked
- When you plan to switch back to a crates.io version
This will help future maintainers understand why a git dependency is being used and when it can be replaced.
📝 Example of improved documentation
-# # Logging and Metrics -# opentelemetry-proto = { version = "0.30.0", features = [ -# "gen-tonic", -# "with-serde", -# "logs", -# "metrics", -# "trace", -# ] } - -# add custom branch with fix until it gets merged +# Logging and Metrics +# Using git dependency temporarily for fix in PR `#XXXX` (link to PR) +# TODO: Switch back to crates.io version once fix is released (tracking issue: `#YYYY`) opentelemetry-proto = { git = "https://github.com/open-telemetry/opentelemetry-rust/", rev = "b096b70b2ffe9beb65a716cf47d5e5db80a9e930", features = [🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Cargo.toml` around lines 91 - 99, The comment near the opentelemetry-proto git dependency is too vague—update the Cargo.toml comment adjacent to the opentelemetry-proto entry to state the exact bug or behavior the custom branch fixes (e.g., describe the failing feature or function), include a link to the upstream issue or PR you’re tracking, and note the target crates.io version or a timeline/condition (e.g., "switch back when upstream vX.Y.Z is released" or "when PR `#123` is merged"); reference the opentelemetry-proto dependency declaration so future maintainers can easily find and replace the git override.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@Cargo.toml`:
- Around line 100-106: The comment about pinning opentelemetry-proto to rev
b096b70b2ffe9beb65a716cf47d5e5db80a9e930 is misleading; update the Cargo.toml
comment near the opentelemetry-proto dependency to state whether this rev is a
temporary workaround (describe the missing fix and expected unpin
condition/timeframe) or the intended upstream bump to 0.10.0 (state that it
tracks the upstream version bump), so future readers know why the git rev is
used for opentelemetry-proto and when it can be removed or converted to a crate
version.
---
Nitpick comments:
In `@Cargo.toml`:
- Around line 91-99: The comment near the opentelemetry-proto git dependency is
too vague—update the Cargo.toml comment adjacent to the opentelemetry-proto
entry to state the exact bug or behavior the custom branch fixes (e.g., describe
the failing feature or function), include a link to the upstream issue or PR
you’re tracking, and note the target crates.io version or a timeline/condition
(e.g., "switch back when upstream vX.Y.Z is released" or "when PR `#123` is
merged"); reference the opentelemetry-proto dependency declaration so future
maintainers can easily find and replace the git override.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: bfbf95c6-139f-46e7-85d1-380b1676e52c
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
Cargo.toml
Summary by CodeRabbit