Conversation
Reviewer's GuideThis PR unifies the application constructors by replacing Class diagram for Envelope and Packet trait updatesclassDiagram
class Envelope {
+id: u32
+msg: Vec<u8>
+new(id: u32, msg: Vec<u8>) Self
+into_parts(self) (u32, Vec<u8>)
}
class Packet {
<<interface>>
+id() u32
+into_parts(self) (u32, Vec<u8>)
+from_parts(id: u32, msg: Vec<u8>) Self
}
Envelope ..|> Packet
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary by CodeRabbit
WalkthroughUpdate the API for Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WireframeApp
participant Envelope (custom)
User->>WireframeApp: WireframeApp::<_, _, MyEnv>::new()
WireframeApp->>Envelope: Use MyEnv (implements Packet)
WireframeApp-->>User: App instance (with custom envelope)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🧰 Additional context used📓 Path-based instructions (1)**/*.rs📄 CodeRabbit Inference Engine (AGENTS.md)
Files:
⚙️ CodeRabbit Configuration File
Files:
🧬 Code Graph Analysis (2)tests/wireframe_protocol.rs (1)
tests/routes.rs (1)
🔇 Additional comments (8)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
README.md(2 hunks)examples/echo.rs(2 hunks)examples/metadata_routing.rs(2 hunks)examples/ping_pong.rs(1 hunks)src/app.rs(3 hunks)tests/lifecycle.rs(4 hunks)tests/response.rs(4 hunks)tests/routes.rs(3 hunks)tests/wireframe_protocol.rs(4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
⚙️ CodeRabbit Configuration File
**/*.rs: * Seek to keep the cyclomatic complexity of functions no more than 12.
Adhere to single responsibility and CQRS
Place function attributes after doc comments.
Do not use
returnin single-line functions.Move conditionals with >2 branches into a predicate function.
Avoid
unsafeunless absolutely necessary.Every module must begin with a
//!doc comment that explains the module's purpose and utility.Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar
Lints must not be silenced except as a last resort.
#[allow]is forbidden.- Only narrowly scoped
#[expect(lint, reason = "...")]is allowed.- No lint groups, no blanket or file-wide suppression.
- Include
FIXME:with link if a fix is expected.Use
rstestfixtures for shared setup and to avoid repetition between tests.Replace duplicated tests with
#[rstest(...)]parameterised cases.Prefer
mockallfor mocks/stubs.Prefer
.expect()over.unwrap()Ensure that any API or behavioural changes are reflected in the documentation in
docs/Ensure that any completed roadmap steps are recorded in the appropriate roadmap in
docs/Files must not exceed 400 lines in length
- Large modules must be decomposed
- Long match statements or dispatch tables should be decomposed by domain and collocated with targets
- Large blocks of inline data (e.g., test fixtures, constants or templates) must be moved to external files and inlined at compile-time or loaded at run-time.
Files:
tests/wireframe_protocol.rsexamples/metadata_routing.rstests/response.rsexamples/ping_pong.rsexamples/echo.rstests/routes.rstests/lifecycle.rssrc/app.rs
**/*.md
⚙️ CodeRabbit Configuration File
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-GB-oxendict (-ize / -our) spelling and grammar
- Paragraphs and bullets must be wrapped to 80 columns, except where a long URL would prevent this (in which case, silence MD013 for that line)
- Code blocks should be wrapped to 120 columns.
- Headings must not be wrapped.
- Documents must start with a level 1 heading
- Headings must correctly increase or decrease by no more than one level at a time
- Use GitHub-flavoured Markdown style for footnotes and endnotes.
- Numbered footnotes must be numbered by order of appearance in the document.
Files:
README.md
🧬 Code Graph Analysis (4)
tests/wireframe_protocol.rs (2)
src/hooks.rs (2)
before_send(32-32)before_send(101-105)src/app.rs (2)
new(203-205)new(290-292)
examples/echo.rs (1)
src/app.rs (2)
new(203-205)new(290-292)
tests/routes.rs (2)
tests/lifecycle.rs (3)
id(109-111)into_parts(113-115)from_parts(117-119)src/app.rs (9)
id(181-181)id(215-217)into_parts(184-184)into_parts(209-211)into_parts(219-221)from_parts(187-187)from_parts(223-225)new(203-205)new(290-292)
tests/lifecycle.rs (2)
src/app.rs (9)
new(203-205)new(290-292)id(181-181)id(215-217)into_parts(184-184)into_parts(209-211)into_parts(219-221)from_parts(187-187)from_parts(223-225)tests/routes.rs (3)
id(28-30)into_parts(32-34)from_parts(36-38)
🪛 GitHub Actions: CI
tests/wireframe_protocol.rs
[warning] 14-34: Prettier formatting check failed. Code style differences detected; import statements and function bodies should be formatted consistently as single lines or multiline.
tests/routes.rs
[warning] 25-25: Prettier formatting check failed. Code style differences detected; function implementations should be formatted as single lines.
tests/lifecycle.rs
[warning] 106-106: Prettier formatting check failed. Code style differences detected; function implementations should be formatted as single lines.
src/app.rs
[warning] 140-140: Prettier formatting check failed. Code style differences detected; lines should be formatted to a single line instead of multiple lines.
[warning] 200-200: Prettier formatting check failed. Code style differences detected; lines should be formatted to a single line instead of multiple lines.
[warning] 209-209: Prettier formatting check failed. Code style differences detected; lines should be formatted to a single line instead of multiple lines.
[warning] 215-215: Prettier formatting check failed. Code style differences detected; lines should be formatted to a single line instead of multiple lines.
[warning] 276-284: Prettier formatting check failed. Code style differences detected; multiline struct and function definitions should be formatted as single lines or vice versa.
[warning] 301-301: Prettier formatting check failed. Code style differences detected; function body should be formatted as a single line.
🪛 GitHub Check: build-test
src/app.rs
[warning] 215-215:
Diff in /home/runner/work/wireframe/wireframe/src/app.rs
[warning] 209-209:
Diff in /home/runner/work/wireframe/wireframe/src/app.rs
[warning] 301-301:
Diff in /home/runner/work/wireframe/wireframe/src/app.rs
[warning] 284-284:
Diff in /home/runner/work/wireframe/wireframe/src/app.rs
[warning] 276-276:
Diff in /home/runner/work/wireframe/wireframe/src/app.rs
🔇 Additional comments (15)
examples/ping_pong.rs (1)
133-133: LGTM! Explicit type annotation improves clarity.The change from
|_|to|_: &Envelope|makes the closure signature explicit and aligns with the consistent pattern applied across other examples.examples/metadata_routing.rs (1)
70-70: LGTM! Explicit envelope type annotations enhance clarity.The addition of explicit
&Envelopetype annotations to the closure parameters improves code readability and maintains consistency with similar changes across other examples.Also applies to: 79-79
examples/echo.rs (2)
8-11: LGTM! Import formatting and envelope type addition support the API update.The multiline import format and explicit
Envelopeimport align with the unified generic constructor approach.
20-20: LGTM! Explicit envelope type annotation improves consistency.The change from
_to_: &Envelopefollows the pattern established across other examples and enhances type clarity.tests/lifecycle.rs (2)
52-52: LGTM! Constructor change aligns with unified API.The replacement of
new_with_envelope()with the genericnew()constructor follows the deprecation plan outlined in the PR objectives.
77-77: LGTM! Explicit generic parameters improve type clarity.The explicit
WireframeApp::<_, _, Envelope>::new()syntax clearly specifies the envelope type and aligns with the new generic constructor approach.Also applies to: 92-92
tests/wireframe_protocol.rs (2)
53-53: LGTM! Explicit generic parameters align with unified API.The explicit
WireframeApp::<_, _, Envelope>::new()syntax correctly uses the new generic constructor approach and improves type clarity.Also applies to: 77-77
37-39: Revert to single-line function formatting.The coding guidelines state "Do not use
returnin single-line functions" and the pipeline failure confirms formatting violations. This method should remain as a single-line implementation.Apply this diff to fix the formatting:
- fn before_send(&self, frame: &mut Self::Frame, _ctx: &mut ConnectionContext) { - frame.push(1); - } + fn before_send(&self, frame: &mut Self::Frame, _ctx: &mut ConnectionContext) { frame.push(1); }Likely an incorrect or invalid review comment.
README.md (1)
95-96: Documentation accurately reflects the unified constructor API.The updates correctly demonstrate the new
WireframeApp::<_, _, MyEnv>::new()syntax, replacing the deprecatednew_with_envelope()method. The examples clearly show how to specify custom envelope types.Also applies to: 110-110
tests/response.rs (1)
9-9: Test updates correctly adopt the unified constructor API.The explicit import of
Envelopeand consistent use ofWireframeApp::<_, _, Envelope>::new()properly aligns with the new generic constructor pattern whilst preserving all test functionality.Also applies to: 42-42, 134-134, 193-193
tests/routes.rs (2)
49-49: Constructor updates correctly use the unified API.The transition from
new_with_envelope()toWireframeApp::<_, _, TestEnvelope>::new()properly implements the new generic constructor pattern.Also applies to: 88-88
28-38: Revert formatting changes to comply with coding guidelines.The Packet trait methods should remain as single-line expressions without explicit return statements, per the coding guidelines: "Do not use
returnin single-line functions."Apply this diff to restore the correct formatting:
- fn id(&self) -> u32 { - self.id - } + fn id(&self) -> u32 { self.id } - fn into_parts(self) -> (u32, Vec<u8>) { - (self.id, self.msg) - } + fn into_parts(self) -> (u32, Vec<u8>) { (self.id, self.msg) } - fn from_parts(id: u32, msg: Vec<u8>) -> Self { - Self { id, msg } - } + fn from_parts(id: u32, msg: Vec<u8>) -> Self { Self { id, msg } }Likely an incorrect or invalid review comment.
src/app.rs (3)
262-265: Excellent unification of the constructor API with comprehensive documentation.The generic implementation with
E: Packetconstraint elegantly unifies the constructor interface. The detailed documentation with practical examples clearly demonstrates the new usage pattern for custom envelope types.Also applies to: 270-292
304-306: Proper deprecation strategy for the old constructor.The deprecated method correctly delegates to the new
new()method whilst providing clear migration guidance to users.
290-292: Consider expanding the constructor implementation.Whilst the current implementation simply returns
Self::default(), the comprehensive documentation and error-returning signature suggest future extensibility. The current approach is sound for maintaining API consistency.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
1 participant
Summary
WireframeApp::newandnew_with_envelopeinto one genericnewnew_with_envelopeas a wrapper aroundnewTesting
make fmtmake lintmake testmake nixie(fails: error: too many arguments. Expected 0 arguments but got 1)https://chatgpt.com/codex/tasks/task_e_688fd388dc08832299c95dd15ab3a723
Summary by Sourcery
Consolidate application constructors by introducing a generic new method parameterized by an envelope type, deprecating new_with_envelope, and propagate the change across docs, tests, and examples.
Enhancements:
Documentation:
Tests:
Chores: