-
Notifications
You must be signed in to change notification settings - Fork 125
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
Change Rust protobufs crate name, license reference #2541
Conversation
WalkthroughThe pull request introduces several changes primarily to the Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
v4-proto-rs/Cargo.toml (1)
5-6
: Consider using a more specific repository URL.While the current repository URL is correct, consider pointing to the specific directory within the v4-chain repository where this crate resides for better discoverability.
-repository = "https://github.com/dydxprotocol/v4-chain" +repository = "https://github.com/dydxprotocol/v4-chain/tree/main/v4-proto-rs"v4-proto-rs/README.md (2)
Line range hint
1-100
: Consider enhancing the documentation.While the core content is good, consider adding these improvements to make the documentation more complete:
- Add a version badge to show the latest published version
- Add an example showing how to use the protobuf types
- Document the SPDX license identifier
LicenseRef-dYdX-Custom
and what it means for usersExample version badge:
[![Crates.io](https://img.shields.io/crates/v/dydx-proto-rust.svg)](https://crates.io/crates/dydx-proto-rust)Would you like me to provide a complete example of these documentation improvements?
The directory should be renamed to match the crate name
dydx-proto-rust
The verification confirms that while the crate has been renamed to
dydx-proto-rust
in the package configuration files (Cargo.toml
,deny.toml
), the directory structure still uses the old namev4-proto-rs
. This inconsistency should be addressed by renaming the directory to match the new crate name.
v4-proto-rs/
directory should be renamed todydx-proto-rust/
- The following files need to be moved:
v4-proto-rs/README.md
v4-proto-rs/deny.toml
v4-proto-rs/Cargo.toml
🔗 Analysis chain
Line range hint
1-1
: Update the README location to match the new crate name.The README is still located in the
v4-proto-rs
directory, but the crate has been renamed todydx-proto-rust
. Consider moving this file to match the new crate name.Let's verify if there are any other references to the old name in the repository:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining references to the old crate name rg "v4-proto-rs" --type-not gitLength of output: 59
Script:
#!/bin/bash # Search for any remaining references to the old crate name without the git type exclusion rg "v4-proto-rs" # Also search for the new crate name to understand the context rg "dydx-proto-rust" # List all README files to understand the directory structure fd "README.md"Length of output: 1303
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- v4-proto-rs/Cargo.toml (1 hunks)
- v4-proto-rs/LICENSE (1 hunks)
- v4-proto-rs/README.md (1 hunks)
- v4-proto-rs/deny.toml (2 hunks)
✅ Files skipped from review due to trivial changes (1)
- v4-proto-rs/LICENSE
🔇 Additional comments (6)
v4-proto-rs/Cargo.toml (2)
7-7
: Verify license file and documentation.The license identifier follows SPDX format for custom licenses. However, we should ensure proper documentation is in place.
#!/bin/bash # Check for LICENSE file if [ -f "LICENSE" ]; then echo "✓ LICENSE file exists" else echo "⚠ LICENSE file missing" fi # Check for license documentation in README if grep -q "LicenseRef-dYdX-Custom" README.md; then echo "✓ License documented in README" else echo "⚠ Consider documenting the custom license in README.md" fi
2-2
: Verify package name availability on crates.io.The new name
dydx-proto-rust
is more descriptive and follows Rust naming conventions. However, since this is preparation for publishing, we should verify the name availability.✅ Verification successful
✓ Package name
dydx-proto-rust
is available on crates.ioThe new package name follows Rust naming conventions and is currently available on crates.io. The name change is appropriate for publishing and aligns well with the organization's branding.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the crate name is already taken on crates.io curl -s https://crates.io/api/v1/crates/dydx-proto-rust | grep -q "errors" if [ $? -eq 0 ]; then echo "✓ Crate name 'dydx-proto-rust' is available" else echo "⚠ Crate name 'dydx-proto-rust' is already taken" fiLength of output: 235
Script:
#!/bin/bash # Retry the crates.io check with better error handling response=$(curl -s https://crates.io/api/v1/crates/dydx-proto-rust) if echo "$response" | jq -e '.errors' >/dev/null 2>&1; then if echo "$response" | jq -e '.errors[] | select(.detail | contains("Not Found"))' >/dev/null 2>&1; then echo "✓ Crate name 'dydx-proto-rust' is available" else echo "⚠ Error checking crate name: $(echo "$response" | jq -r '.errors[0].detail')" fi else echo "⚠ Crate name 'dydx-proto-rust' is already taken" echo "Details: $(echo "$response" | jq -r '.crate.description // "No description"')" fiLength of output: 611
v4-proto-rs/deny.toml (3)
28-28
: LGTM: Custom license follows SPDX formatThe addition of
"LicenseRef-dYdX-Custom"
to the allowed licenses list follows the SPDX specification for custom licenses.
64-64
: LGTM: Explicit source configurationThe addition of
bitbucket = []
makes the source configuration more explicit by clearly indicating that no Bitbucket repositories are allowed.
33-39
: Verify LICENSE file hashThe license clarification block is well-structured, but please verify that the LICENSE file hash (0x30bcd1e3) matches the actual file content.
v4-proto-rs/README.md (1)
9-9
: LGTM: Dependency name updated correctly.The dependency name has been updated to match the new crate name
dydx-proto-rust
.
Changelist
Final changes before crate publishing:
dydx-proto-rust
,license
value change to:LicenseRef-dYdX-Custom
, following SPDX rules. Clarified indeny.toml
.Test Plan
[Describe how this PR was tested (if applicable)]
Author/Reviewer Checklist
state-breaking
label.indexer-postgres-breaking
label.PrepareProposal
orProcessProposal
, manually add the labelproposal-breaking
.feature:[feature-name]
.backport/[branch-name]
.refactor
,chore
,bug
.Summary by CodeRabbit
New Features
Documentation
Chores