-
Notifications
You must be signed in to change notification settings - Fork 21
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 dependencies and refactor for no-std #132
Conversation
Streamline dependencies and improve error handling by replacing generic with specific error types. Restructure feature flags (`parse_price`, `extensions`) to improve modularity and `no-std` compatibility. Bump SDK version to `3.6.0` and consolidate documentation for better clarity.
WalkthroughThis pull request introduces a new GitHub Actions build step to explicitly compile with the Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant CI as GitHub Actions
Dev->>CI: Push commit
CI->>CI: Run "Build" step (cargo build)
CI->>CI: Run "Build with std feature" (cargo build --features std)
CI->>CI: Run "Build with extensions" step
CI->>Dev: Report build status
Possibly related PRs
Poem
Tip 🌐 Web search-backed reviews and chat
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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (14)
.github/workflows/rust.yml
(1 hunks)Cargo.toml
(2 hunks)README.md
(1 hunks)src/entities/pool.rs
(0 hunks)src/entities/tick_list_data_provider.rs
(1 hunks)src/entities/trade.rs
(2 hunks)src/extensions/position.rs
(5 hunks)src/extensions/price_tick_conversions.rs
(4 hunks)src/lib.rs
(1 hunks)src/multicall.rs
(1 hunks)src/tests.rs
(1 hunks)src/utils/sqrt_price_math.rs
(1 hunks)src/utils/swap_math.rs
(1 hunks)src/utils/types.rs
(1 hunks)
💤 Files with no reviewable changes (1)
- src/entities/pool.rs
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Rust Tests
🔇 Additional comments (17)
src/tests.rs (1)
5-5
: Bazinga! This change is theoretically sound.The addition of
pub(crate) use alloc::vec
is a logical step in achieving no-std compatibility, making the vector functionality available to other test modules within the crate.src/entities/tick_list_data_provider.rs (1)
20-20
: As expected, a perfectly logical addition.The import of
alloc::vec
in the test module aligns with the theoretical framework of no-std compatibility.src/multicall.rs (1)
59-59
: This modification exhibits remarkable consistency.The addition of
alloc::vec
maintains the theoretical consistency of no-std support across the test modules.src/utils/sqrt_price_math.rs (1)
475-475
: Fascinating! A slight variation in import syntax, yet equally valid.The direct import of
Vec
fromalloc::vec
demonstrates an acceptable alternative approach to achieving no-std compatibility.src/lib.rs (2)
1-1
: Bazinga! Excellent documentation strategy.Including the README.md content as documentation ensures a single source of truth for project documentation.
2-2
: Fascinating conditional compilation modification.The updated cfg_attr ensures proper no_std support while allowing extensions during testing. This is a theoretically sound approach to feature management.
src/utils/types.rs (1)
3-3
: Logical import consolidation.Importing BigDecimal from uniswap_sdk_core::prelude reduces external dependencies, which is consistent with our goal of dependency streamlining.
src/extensions/price_tick_conversions.rs (2)
46-46
: Excellent feature flag implementation.The parse_price feature flag properly isolates this functionality, adhering to the principle of modular feature management.
52-52
: Superior error handling implementation.The switch to anyhow::Result and proper error propagation with the ? operator significantly improves error handling robustness.
Also applies to: 60-62
src/extensions/position.rs (1)
12-12
: Theoretically sound error type specification.The explicit Result<T, Error> return types and TransportError import provide better error handling clarity and type safety.
Also applies to: 265-265, 363-363
Cargo.toml (3)
42-47
: Excellent addition of the parse_price feature!The new feature logically groups related dependencies (anyhow, regex) and maintains proper feature hierarchy by depending on 'extensions' and 'std'.
48-58
: Comprehensive std feature organization.The std feature now properly enables std support for all relevant dependencies. The use of the
?
operator in feature flags likealloy?/std
is particularly elegant as it makes the feature optional.
3-3
: Verify semantic versioning compliance for version bump.The version increment from 3.5.1 to 3.6.0 suggests new functionality. Based on the changes in the PR, particularly the addition of the
parse_price
feature, this appears to be a correct minor version bump.✅ Verification successful
🌐 Web query:
What constitutes a minor version bump in semantic versioning?
💡 Result:
A minor version bump in semantic versioning occurs when **backwards-compatible functionality is added** to a software project without introducing breaking changes[2][4][5]. Here's how it works: ### Key Criteria for Minor Version Increments - **New features added**: Introducing functionality that doesn’t break existing API usage[2][5]. - **Deprecations**: Marking existing API elements for future removal (without deleting them yet)[5]. - **Internal improvements**: Substantial non-breaking codebase changes (optional)[4]. ### Rules from SemVer Specifications 1. Minor versions (**Y** in `X.Y.Z`) are reset to `0` when: - A major version (X) is incremented[4][5]. 2. Patch versions (**Z**) reset to `0` after a minor bump[4][5]. ### Example Workflow | Current Version | Change Type | New Version | |-----------------|-------------------------|-------------| | `1.2.4` | Add search API feature | `1.3.0` | | `2.0.1` | Deprecate old auth method | `2.1.0` | ### Edge Case Clarification - Removing deprecated functionality **always requires a major version bump**, even if previously marked for removal in an earlier minor release[1][5]. For example: - Deprecate feature X in `1.5.0` (minor bump) - Remove feature X in `2.0.0` (major bump) This system ensures developers can safely update dependencies while understanding compatibility risks through version numbers[2][5]. Citations: - 1: https://github.com/semver/semver/issues/712 - 2: https://arjancodes.com/blog/how-to-implement-semantic-versioning-in-software-projects/ - 3: https://gatk.broadinstitute.org/hc/en-us/articles/360035890651-Version-numbers - 4: https://semver.org/spec/v1.0.0.html - 5: https://semver.org - 6: https://gitversion.net/docs/reference/version-increments - 7: https://semver.org/spec/v2.0.0-rc.1.html - 8: https://discourse.julialang.org/t/questions-on-the-versioning-semantics-of-julia-packages/109926
Semantic Versioning Compliance Verified
The version bump from 3.5.1 to 3.6.0 is semver-compliant since the new functionality—the addition of the
parse_price
feature—represents a backward-compatible feature enhancement..github/workflows/rust.yml (1)
62-63
: Logical placement of std feature build step!The addition of this step ensures that the std feature compiles correctly. Its placement between the default build and extensions build follows a natural progression of complexity.
README.md (1)
74-89
: Appropriate use of rust,ignore attribute.The addition of the 'ignore' attribute to the code block is correct as this example contains async code that can't be validated by rustdoc.
src/entities/trade.rs (1)
3-3
: Excellent migration to alloy_primitives HashSet!The switch from
FxHashSet
toHashSet
from alloy_primitives aligns with the crate's move towards standardization on alloy primitives. This change maintains functionality while reducing external dependencies.Also applies to: 191-191
src/utils/swap_math.rs (1)
169-173
: Simplified Default Value Assignment forsqrt_price_limit_x96
Using
unwrap_or
with an inlineif
...else
cleans up the code nicely without altering its logic. Althoughunwrap_or
eagerly computes the default value, here it involves only trivial arithmetic on constants—hardly a performance concern. Bazinga!
Streamline dependencies and improve error handling by replacing generic with specific error types. Restructure feature flags (
parse_price
,extensions
) to improve modularity andno-std
compatibility. Bump SDK version to3.6.0
and consolidate documentation for better clarity.Summary by CodeRabbit