Deprecate SharedState::new #41
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's GuideDeprecate SharedState::new in favor of using the From<Arc> conversion via into(), update all examples and docs to reflect the new construction pattern, and record the change in the CHANGELOG. Sequence Diagram: SharedState Creation via .into()sequenceDiagram
actor Developer
participant MyArcT as "my_arc: Arc<T>"
participant SharedStateType as "SharedState<T> (type)"
Developer->>MyArcT: .into()
activate MyArcT
MyArcT->>SharedStateType: ::from(my_arc)
deactivate MyArcT
activate SharedStateType
SharedStateType-->>Developer: new_shared_state_instance
deactivate SharedStateType
Updated Class Diagram for SharedStateclassDiagram
class SharedState~T~ {
<<Struct>>
#inner: Arc~T~
+new(inner: Arc~T~) SharedState~T~ "<<deprecated since 0.2.0, construct via inner.into() instead>>"
+from(inner: Arc~T~) SharedState~T~
+deref() &T
}
class Arc~T~ {
<<Struct>>
+into() SharedState~T~
}
Arc~T~ ..> SharedState~T~ : .into() calls SharedState.from()
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded@leynos has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 5 minutes and 39 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
There was a problem hiding this comment.
Gates Passed
See analysis details in CodeScene
Absence of Expected Change Pattern
- wireframe/src/extractor.rs is usually changed with: wireframe/src/server.rs, wireframe/src/middleware.rs
Quality Gate Profile: Pay Down Tech Debt
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes - here's some feedback:
- Add a
sincefield to the#[deprecated]attribute to provide clearer deprecation metadata. - Double-check there are no remaining references to
SharedState::newin tests or examples to ensure full migration toinner.into().
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Add a `since` field to the `#[deprecated]` attribute to provide clearer deprecation metadata.
- Double-check there are no remaining references to `SharedState::new` in tests or examples to ensure full migration to `inner.into()`.
## Individual Comments
### Comment 1
<location> `src/extractor.rs:84` </location>
<code_context>
+ /// let shared: SharedState<u32> = state.clone().into();
/// assert_eq!(*shared, 42);
/// ```
+ #[deprecated(note = "construct via `inner.into()` instead")]
pub fn new(inner: Arc<T>) -> Self {
Self(inner)
</code_context>
<issue_to_address>
Include `since` in `#[deprecated]` attribute
Adding a `since` version clarifies when the deprecation was introduced.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
#[deprecated(note = "construct via `inner.into()` instead")]
=======
#[deprecated(since = "0.3.0", note = "construct via `inner.into()` instead")]
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Gates Passed
See analysis details in CodeScene
Absence of Expected Change Pattern
- wireframe/src/extractor.rs is usually changed with: wireframe/src/server.rs, wireframe/src/middleware.rs
Quality Gate Profile: Pay Down Tech Debt
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes - here's some feedback:
- The markdownlint run is failing with hundreds of errors—please fix or suppress these so the docs build cleanly in CI.
- Double-check for any leftover
SharedState::newusages (in examples, tests, etc.) and update them to the.into()constructor to keep things consistent.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The markdownlint run is failing with hundreds of errors—please fix or suppress these so the docs build cleanly in CI.
- Double-check for any leftover `SharedState::new` usages (in examples, tests, etc.) and update them to the `.into()` constructor to keep things consistent.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Gates Passed
See analysis details in CodeScene
Absence of Expected Change Pattern
- wireframe/src/extractor.rs is usually changed with: wireframe/src/server.rs, wireframe/src/middleware.rs
Quality Gate Profile: Pay Down Tech Debt
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
SharedState::newas deprecatedSharedStateviaFromimplsCHANGELOGTesting
cargo clippy -- -D warningsRUSTFLAGS="-D warnings" cargo testmarkdownlint README.md docs/*.md CHANGELOG.md(fails: 297 errors)nixie README.md docs/*.mdhttps://chatgpt.com/codex/tasks/task_e_684e35de1fbc8322a8b8df5c164be392
Summary by Sourcery
Deprecate the SharedState::new constructor in favor of the From impl and update documentation and code examples accordingly
Enhancements:
Documentation: