Update README with WireframeProtocol integration#152
Conversation
Reviewer's GuideUpdates README to document the new unified WireframeProtocol trait for connection lifecycle callbacks and refreshes the example to demonstrate registering a protocol via with_protocol. Class diagram for the new WireframeProtocol traitclassDiagram
class WireframeProtocol {
<<trait>>
+Frame: FrameLike
+ProtocolError
+on_connection_setup(handle: PushHandle<Frame>, ctx: &mut ConnectionContext)
+before_send(frame: &mut Frame, ctx: &mut ConnectionContext)
+on_command_end(ctx: &mut ConnectionContext)
}
class PushHandle {
}
class FrameLike {
}
class ConnectionContext {
}
WireframeProtocol ..> PushHandle : uses
WireframeProtocol ..> FrameLike : uses
WireframeProtocol ..> ConnectionContext : uses
Class diagram for WireframeApp protocol registrationclassDiagram
class WireframeApp {
+with_protocol(protocol: impl WireframeProtocol)
}
class WireframeProtocol {
}
WireframeApp ..> WireframeProtocol : registers
File-Level Changes
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 0 minutes and 38 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 (1)
Summary by CodeRabbit
WalkthroughThe documentation for Changes
Sequence Diagram(s)sequenceDiagram
participant App as WireframeApp
participant Protocol as WireframeProtocol
participant Conn as ConnectionContext
participant Frame as Frame
App->>Protocol: with_protocol(MyProtocol)
App->>Conn: on new connection
Protocol->>Conn: on_connection_setup(handle, ctx)
App->>Frame: prepare frame
Protocol->>Frame: before_send(frame, ctx)
App->>Conn: send frame
App->>Conn: command completes
Protocol->>Conn: on_command_end(ctx)
Possibly related PRs
Poem
✨ 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. 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 (
|
![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:
- The trait methods aren’t marked async or returning a future yet the previous callbacks were async—consider clarifying how asynchronous setup/teardown is handled in this new trait.
- You define
type ProtocolErrorbut don’t reference it in any of the callbacks—either integrate error handling into these methods or remove the unused associated type. - Adding a minimal example of a
MySqlProtocolImplstruct and itsWireframeProtocolimpl would help illustrate howwith_protocolties a custom protocol into the app.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The trait methods aren’t marked async or returning a future yet the previous callbacks were async—consider clarifying how asynchronous setup/teardown is handled in this new trait.
- You define `type ProtocolError` but don’t reference it in any of the callbacks—either integrate error handling into these methods or remove the unused associated type.
- Adding a minimal example of a `MySqlProtocolImpl` struct and its `WireframeProtocol` impl would help illustrate how `with_protocol` ties a custom protocol into the app.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
README.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.md`: Validate Markdown files using `markdownlint *.md **/*.md`. Run `mdfor...
**/*.md: Validate Markdown files usingmarkdownlint *.md **/*.md.
Runmdformat-allafter any documentation changes to format all Markdown files and fix table markup.
Validate Markdown Mermaid diagrams using thenixieCLI by runningnixie *.md **/*.md.
Markdown paragraphs and bullet points must be wrapped at 80 columns.
Code blocks in Markdown files must be wrapped at 120 columns.
Tables and headings in Markdown files must not be wrapped.
📄 Source: CodeRabbit Inference Engine (AGENTS.md)
List of files the instruction was applied to:
README.md
`**/*.md`: * Avoid 2nd person or 1st person pronouns ("I", "you", "we") * Use en...
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-oxendic 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.
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
README.md
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: leynos/wireframe#0
File: docs/asynchronous-outbound-messaging-design.md:0-0
Timestamp: 2025-06-28T23:56:14.514Z
Learning: All protocol-specific logic and callbacks should be encapsulated in a single WireframeProtocol trait, which defines hooks for connection setup, frame mutation before send, and command completion. This trait-based approach is preferred over ad-hoc closures for extensibility and maintainability.
README.md (1)
Learnt from: CR
PR: leynos/wireframe#0
File: docs/asynchronous-outbound-messaging-design.md:0-0
Timestamp: 2025-06-28T23:56:14.514Z
Learning: All protocol-specific logic and callbacks should be encapsulated in a single WireframeProtocol trait, which defines hooks for connection setup, frame mutation before send, and command completion. This trait-based approach is preferred over ad-hoc closures for extensibility and maintainability.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-test
🔇 Additional comments (1)
README.md (1)
136-140: README.md paragraph wrapped to 80 columns (MD013 cleared)The updated paragraph on lines 136–140 now complies with the 80-column wrap limit and passes
markdownlintMD013. No further changes are required.
leynos
deleted the
codex/add-wireframeprotocol-interface-details-to-readme
branch
1 participant
Summary
WireframeProtocoltrait in the Connection Lifecycle sectionwith_protocolTesting
make fmtmake lintmake testmarkdownlint README.md docs/asynchronous-outbound-messaging-design.mdnixie README.md docs/asynchronous-outbound-messaging-design.md(fails: KeyboardInterrupt)https://chatgpt.com/codex/tasks/task_e_686088eb3a5c8322a2dad1fc34dcc5ba
Summary by Sourcery
Update README to describe the new WireframeProtocol trait for connection lifecycle callbacks and show protocol registration using with_protocol
Documentation: