-
Notifications
You must be signed in to change notification settings - Fork 2
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(engine): update Rust version to 1.83 and adjust Dockerfiles #667
Conversation
WalkthroughThis pull request primarily updates the Changes
Possibly related PRs
Suggested labels
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
Documentation and Community
|
✅ Deploy Preview for reearth-flow canceled.
|
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
🧹 Outside diff range and nitpick comments (1)
engine/runtime/geometry/src/types/line_string.rs (1)
Line range hint
1-1
: Excellent systematic modernization of lifetime handling.The changes across all files demonstrate a well-executed systematic modernization of lifetime parameters, leveraging Rust 1.83's improved lifetime elision rules. This improves code maintainability while preserving all functionality.
Key benefits:
- Reduced verbosity
- Improved readability
- Consistent modern style
- No semantic changes
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
engine/Cargo.lock
is excluded by!**/*.lock
,!**/*.lock
📒 Files selected for processing (17)
engine/Cargo.toml
(6 hunks)engine/Makefile.toml
(1 hunks)engine/clippy.toml
(1 hunks)engine/containers/cli/Dockerfile
(1 hunks)engine/containers/worker/Dockerfile
(1 hunks)engine/runtime/geometry/src/algorithm/coordinate_position.rs
(1 hunks)engine/runtime/geometry/src/algorithm/coords_iter.rs
(4 hunks)engine/runtime/geometry/src/algorithm/euclidean_distance.rs
(0 hunks)engine/runtime/geometry/src/algorithm/euclidean_length.rs
(0 hunks)engine/runtime/geometry/src/algorithm/relate/geomgraph/index/rstar_edge_set_intersector.rs
(1 hunks)engine/runtime/geometry/src/algorithm/sweep/cross.rs
(1 hunks)engine/runtime/geometry/src/algorithm/winding_order.rs
(2 hunks)engine/runtime/geometry/src/types/line_string.rs
(5 hunks)engine/runtime/geometry/src/types/multi_polygon.rs
(1 hunks)engine/runtime/geometry/src/types/polygon.rs
(4 hunks)engine/runtime/runtime/src/dag_schemas.rs
(1 hunks)engine/runtime/telemetry/src/lib.rs
(3 hunks)
💤 Files with no reviewable changes (2)
- engine/runtime/geometry/src/algorithm/euclidean_length.rs
- engine/runtime/geometry/src/algorithm/euclidean_distance.rs
✅ Files skipped from review due to trivial changes (2)
- engine/clippy.toml
- engine/Cargo.toml
🔇 Additional comments (19)
engine/runtime/geometry/src/algorithm/winding_order.rs (2)
Line range hint 21-38
: LGTM! Good simplification of lifetime annotations.
The change to use an elided lifetime ('_
) instead of an explicit lifetime parameter simplifies the code while maintaining the same semantics. This aligns well with modern Rust practices.
Line range hint 39-47
: LGTM! Consistent lifetime simplification.
The ExactSizeIterator
implementation follows the same lifetime simplification pattern as the Iterator
implementation, maintaining consistency throughout the code.
engine/runtime/geometry/src/algorithm/sweep/cross.rs (1)
Line range hint 15-21
: LGTM! Good simplification of lifetime annotations.
The removal of explicit lifetime parameter 'a
from the reference implementation is a good improvement. This change leverages Rust's lifetime elision rules while maintaining the same safety guarantees, making the code more concise and easier to maintain.
engine/containers/worker/Dockerfile (1)
2-2
: Verify Rust 1.83 compatibility across the project.
The base image update to rust:1.83-slim-bookworm
looks good. To ensure smooth deployment:
- Confirm all dependencies support Rust 1.83
- Verify successful builds in CI pipeline
- Check for any performance impact
✅ Verification successful
The initial scan revealed that the workspace is configured to use Rust 1.83 in the main engine/Cargo.toml, but the websocket component specifies 1.81. Let's gather more information about dependencies and toolchain specifications to ensure complete compatibility.
Rust 1.83 update is compatible with the project
The workspace explicitly specifies Rust 1.83 as the minimum supported version in engine/Cargo.toml, and all other crates inherit this version through workspace inheritance. The dependencies in Cargo.lock show no version conflicts that would prevent using Rust 1.83.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Rust version compatibility
# Check if Cargo.toml specifies any version constraints
rg "rust-version|MSRV" -A 2
# Look for any conditional compilation based on Rust version
rg "#\[cfg\(.*rust_version.*\)\]" -A 2
Length of output: 4460
Script:
#!/bin/bash
# Check for any version-specific dependencies in Cargo.toml files
rg "version\s*=\s*\"" engine/Cargo.lock -A 1
# Check for any rust toolchain specifications
fd "rust-toolchain" --type f --exec cat {}
Length of output: 73551
engine/containers/cli/Dockerfile (1)
2-2
: LGTM! Good version consistency across services.
The base image update to rust:1.83-slim-bookworm
maintains consistency with the worker Dockerfile, which is excellent practice for ensuring reliable builds and reducing version-related issues across services.
engine/Makefile.toml (1)
23-23
: Note: Clippy warnings will now block builds
The addition of -- -D warnings
flag makes clippy treat all warnings as errors. This enforces stricter code quality standards but may require addressing existing warnings across the codebase.
Let's check for any existing clippy warnings that might now block builds:
Consider adding this change gradually:
- First, fix existing warnings
- Then enable
-D warnings
flag
engine/runtime/geometry/src/algorithm/relate/geomgraph/index/rstar_edge_set_intersector.rs (1)
44-44
: LGTM: Good lifetime parameter simplification
The change from explicit lifetime 'a
to anonymous lifetime '_
simplifies the code while maintaining the same semantics. This aligns well with Rust's lifetime elision rules.
engine/runtime/runtime/src/dag_schemas.rs (1)
387-387
: LGTM: Improved lifetime annotation clarity
The explicit lifetime parameter in the return type better expresses the relationship between the returned references and the DagSchemas instance.
engine/runtime/geometry/src/types/polygon.rs (3)
Line range hint 394-411
: LGTM: Simplified lifetime annotation
The change to use anonymous lifetime '_
instead of explicit 'a
simplifies the code while maintaining the same semantics. This is in line with modern Rust practices.
Line range hint 412-437
: LGTM: Consistent lifetime annotation simplification
The change maintains consistency with other implementations by using the anonymous lifetime '_
.
Line range hint 438-456
: LGTM: Consistent lifetime annotation pattern
The change follows the established pattern of using anonymous lifetimes, maintaining consistency throughout the codebase.
engine/runtime/geometry/src/algorithm/coords_iter.rs (3)
Line range hint 585-618
: LGTM: Simplified iterator implementation
The iterator implementation has been updated to use anonymous lifetimes, consistent with the project-wide changes.
Line range hint 619-657
: LGTM: Consistent Debug implementation
The Debug trait implementation has been updated to use anonymous lifetimes, maintaining consistency with other changes.
Line range hint 658-724
: LGTM: Consistent implementation patterns
Both Iterator and Debug implementations for GeometryExteriorCoordsIter have been updated to use anonymous lifetimes, maintaining consistency with the codebase-wide changes.
engine/runtime/geometry/src/types/multi_polygon.rs (1)
167-167
: LGTM: Good lifetime parameter simplification.
The change to use anonymous lifetime '_
instead of an explicit lifetime parameter aligns with modern Rust practices and improves code readability without changing the semantics.
engine/runtime/geometry/src/algorithm/coordinate_position.rs (1)
50-50
: LGTM: Consistent lifetime parameter simplification.
The change to use anonymous lifetime '_
for GeometryCow
implementation aligns with the PR's objective to modernize the codebase and matches similar changes in other files.
engine/runtime/geometry/src/types/line_string.rs (3)
Line range hint 113-123
: LGTM: Clean lifetime simplification for PointsIter traits.
The changes to ExactSizeIterator
and DoubleEndedIterator
implementations for PointsIter
appropriately use anonymous lifetimes, improving code clarity.
Line range hint 141-151
: LGTM: Clean lifetime simplification for CoordinatesIter traits.
The changes to ExactSizeIterator
and DoubleEndedIterator
implementations for CoordinatesIter
follow the same pattern of lifetime simplification.
Line range hint 319-328
: LGTM: Consistent lifetime handling in conversion implementations.
The changes to From
trait implementations for NLineString2
and NLineString3
conversions maintain consistency with the lifetime simplification pattern across the codebase.
Also applies to: 342-362
Overview
What I've done
What I haven't done
How I tested
Screenshot
Which point I want you to review particularly
Memo
Summary by CodeRabbit
New Features
EuclideanLength
trait for calculating lengths of geometric entities.Improvements
Bug Fixes
from_graph
method for better traceability during execution.Documentation