-
Notifications
You must be signed in to change notification settings - Fork 39
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
chore: update to latest rust dash core with x11 optional #2251
Conversation
WalkthroughThe pull request introduces updates to several Changes
Possibly related PRs
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 (1)
packages/rs-dpp/Cargo.toml (1)
Line range hint
72-265
: LGTM: Feature updates and new feature groupsThe changes to the features, including the addition of new features to the
all_features
list and the introduction of new feature groups (dash-sdk-features
andall_features_without_client
), align well with the PR objectives. These changes provide more granular control over the package's functionality.Consider updating the package documentation to reflect these new feature groups and their use cases.
Would you like assistance in drafting documentation updates for these new feature groups?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
Cargo.lock
is excluded by!**/*.lock
📒 Files selected for processing (4)
- packages/rs-dpp/Cargo.toml (1 hunks)
- packages/rs-drive-abci/Cargo.toml (1 hunks)
- packages/rs-sdk/Cargo.toml (1 hunks)
- packages/simple-signer/Cargo.toml (1 hunks)
🧰 Additional context used
🔇 Additional comments (6)
packages/simple-signer/Cargo.toml (1)
11-11
: LGTM! Dependency version update looks good.The update of
dashcore-rpc
fromv0.15.4
tov0.15.7
aligns with the PR objective to update to the latest rust Dash Core. This change should resolve the issue with unnecessary X11 compilation.Let's verify if this update is consistent across other packages in the project:
If any inconsistencies are found, please update the other packages accordingly.
✅ Verification successful
Verified:
dashcore-rpc
dependency consistently set tov0.15.7
across all packages.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistency of dashcore-rpc version across the project # Test: Search for dashcore-rpc dependency in all Cargo.toml files # Expect: All occurrences should use v0.15.7 echo "Checking dashcore-rpc versions:" rg --type toml 'dashcore-rpc.*tag.*v0\.15\.' -g 'Cargo.toml'Length of output: 495
packages/rs-drive-abci/Cargo.toml (1)
31-31
: Approved: Version update for dashcore-rpc. Please verify a few points.The update of
dashcore-rpc
fromv0.15.4
tov0.15.7
aligns with the PR objective of updating to the latest rust Dash Core. However, there are a few points to verify:
Could you please confirm that there are no breaking changes in this version update by checking the changelog for
dashcore-rpc
?The PR objectives mention X11 becoming optional, but this isn't directly reflected in the
Cargo.toml
change. Is there a specific reason for this, or are there other files where this optionality is implemented?If X11 is now optional, are there any code changes needed in the
drive-abci
package to take advantage of this optionality?To help verify the impact of this change, you can run the following script:
This script will help identify any X11-related code or features that might need to be updated based on the new optionality.
packages/rs-sdk/Cargo.toml (1)
36-36
: Version update for dashcore-rpc dependencyThe version of
dashcore-rpc
has been updated fromv0.15.4
tov0.15.7
. This change aligns with the PR objective to update to the latest rust Dash Core.To ensure this update doesn't introduce any breaking changes or affect other parts of the package, let's run the following verification:
packages/rs-dpp/Cargo.toml (3)
Line range hint
3-3
: LGTM: Version update to 1.4.1The package version update to 1.4.1 aligns with the PR objectives of updating to the latest rust Dash Core. This version bump is appropriate for a non-breaking change.
32-32
: LGTM: Updated dashcore dependency to use a specific tagUpdating the
dashcore
dependency to use a specific tag (0.31.0) instead of the master branch is a good practice. It ensures better stability and reproducibility of builds.
Line range hint
70-70
: Query: once_cell version downgradeThe
once_cell
dev-dependency has been updated from version 1.19.0 to 1.7, which appears to be a downgrade. Can you explain the reasoning behind this change? It's generally recommended to use the latest stable version of dependencies unless there are specific compatibility issues.
Issue being fixed or feature implemented
X11 was being compiled even though it wasn't needed through rust dash core. The new version of rust Dash Core has it as an optional dependency.
What was done?
Updated to latest rust dash core.
How Has This Been Tested?
Breaking Changes
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
New Features
dpp
package for enhanced configuration options.Dependency Updates
dashcore
dependency in thedpp
package to a specific tagged version.once_cell
version in thedpp
package.dashcore-rpc
version across multiple packages (drive-abci
,sdk
,simple-signer
).