Rename payload() to into_payload() across codebase#445
Merged
Conversation
Contributor
Reviewer's GuideRenames the consumptive payload accessor from payload() to into_payload() across core types, updates all call sites to use the new method, and introduces deprecated payload() aliases that forward to into_payload() to preserve backward compatibility and clarify ownership semantics. Class diagram for payload and into_payload on core packet typesclassDiagram
class FragmentParts {
+id() u64
+correlation_id() Option<u64>
+into_payload(self) Vec<u8>
+payload(self) Vec<u8>
}
class PacketParts {
+id() u64
+correlation_id() Option<u64>
+into_payload(self) Vec<u8>
+payload(self) Vec<u8>
}
class Envelope {
+new(id u64, correlation_id Option<u64>, payload Vec<u8>) Envelope
+id(&self) u64
+correlation_id(&self) Option<u64>
+into_parts(self) PacketParts
}
class Fragmentable {
<<trait>>
+into_fragment_parts(self) FragmentParts
+from_fragment_parts(parts FragmentParts) Self
}
class Packet {
<<trait>>
+into_parts(self) PacketParts
+from_parts(parts PacketParts) Self
}
Fragmentable <|.. Packet
PacketParts --> Envelope : converted_to
FragmentParts --> Envelope : used_in_fragmentation
PacketParts o--> FragmentParts : Fragmentable_impl_uses
Envelope ..> PacketParts : From<PacketParts>_uses_into_payload
Fragmentable ..> FragmentParts : uses_into_payload
Fragmentable ..> PacketParts : uses_into_payload
Flow diagram for deprecated payload alias delegating to into_payloadflowchart LR
A[PacketParts_or_FragmentParts_instance] --> B{Caller_method}
B -->|prefers_new_API| C[into_payload]
B -->|legacy_call| D[payload]
C --> E[returns Vec<u8> payload]
D --> F[delegates_to_into_payload]
F --> C
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
Contributor
Summary by CodeRabbit
WalkthroughRename consuming payload accessors to Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/v0-1-0-to-v0-2-0-migration-guide.md`:
- Around line 3-4: Replace the en-GB-oxford-inconsistent word "summarises" in
the document heading sentence ("This guide summarises the breaking changes you
need to address when migrating from wireframe v0.1.0 to v0.2.0.") with the
en-GB-oxendict spelling "summarizes" to conform to the coding guidelines; update
that exact sentence to read "This guide summarizes the breaking changes you need
to address when migrating from wireframe v0.1.0 to v0.2.0."
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/preamble.rs (1)
1-561: 🧹 Nitpick | 🔵 TrivialFile exceeds 400-line limit.
At 561 lines, this file surpasses the 400-line threshold specified in the coding guidelines. Decompose by grouping related tests (e.g., timeout tests, callback override tests, response writing tests) into separate modules or files under a
tests/preamble/directory.As per coding guidelines: "Files must not exceed 400 lines in length. Large modules must be decomposed."
🤖 Fix all issues with AI agents
In `@tests/preamble.rs`:
- Around line 186-196: The test function server_triggers_expected_callback
returns TestResult but uses assert! and assert_eq!, so add the attribute
#[expect(clippy::panic_in_result_fn, reason = "asserts provide clearer
diagnostics in tests")] immediately above the async fn
server_triggers_expected_callback(...) signature to match the existing pattern
used in parse_valid_preamble and invalid_magic_is_error and suppress the clippy
lint while keeping the asserts.
leynos
force-pushed
the
rename-payload-to-into-payload-q2jpyh
branch
from
ca30c55 to
2dce27f
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/envelope.rs (1)
187-198:⚠️ Potential issue | 🟠 MajorAdd the deprecated
payload()alias for backward compatibility.The PR objectives explicitly require a deprecated
payload()alias that delegates tointo_payload()to ensure downstream crates remain compatible during migration. This alias is missing.🔧 Proposed fix: add deprecated alias after into_payload()
#[must_use] pub fn into_payload(self) -> Vec<u8> { self.payload } + + /// Consume the parts and return the raw payload bytes. + /// + /// # Deprecated + /// + /// Use [`into_payload`](Self::into_payload) instead. + #[deprecated(since = "0.2.0", note = "use `into_payload` instead")] + #[must_use] + pub fn payload(self) -> Vec<u8> { self.into_payload() }
…nto_payload() consuming method Replaced the consuming method `payload()` with a new method named `into_payload()` for `PacketParts` and `FragmentParts`. The old `payload()` method is now deprecated and forwards to `into_payload()`. All internal usages are updated to use `into_payload()` for clarity and consistency. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
- Introduces v0.1.0 to v0.2.0 migration guide detailing breaking changes - Notes removal of deprecated payload() methods in favor of into_payload() - Updates existing docs for minor formatting improvements - Removes deprecated payload accessor methods from PacketParts and FragmentParts These changes improve developer experience by documenting required migration steps and cleaning up deprecated APIs. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
Refactored the `server_triggers_expected_callback` test to move the matching on expected callback inside the async block, unifying and slightly increasing the timeout durations for success and failure channels. This improves test clarity and consistency by centralizing assertion logic and using clearer timing boundaries. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
leynos
force-pushed
the
rename-payload-to-into-payload-q2jpyh
branch
from
2dce27f to
efe3695
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/envelope.rs (1)
187-198:⚠️ Potential issue | 🟡 MinorAdd deprecated
payload(self)alias toPacketPartsfor backward compatibility.The PR objectives specify a deprecated
payload(self)alias delegating tointo_payload(), but the implementation omits this. Add the alias with an appropriate#[deprecated]attribute and messaging to support the transition period for downstream crates:#[deprecated(since = "...", note = "use `into_payload()` instead")] pub fn payload(self) -> Vec<u8> { self.into_payload() }This ensures existing code continues to compile whilst guiding users toward the consuming method.
🤖 Fix all issues with AI agents
In `@docs/v0-1-0-to-v0-2-0-migration-guide.md`:
- Around line 19-27: Add a short before/after code snippet showing the payload
accessor rename for both PacketParts and FragmentParts: demonstrate the old call
PacketParts::payload(self) / FragmentParts::payload(self) and the new call
PacketParts::into_payload(self) / FragmentParts::into_payload(self) (e.g., “//
Before ... let payload = parts.payload();” and “// After ... let payload =
parts.into_payload();”) so it matches the configuration builder example style
and appears in the same section.
In `@src/fragment/packet.rs`:
- Around line 51-52: Add a deprecated forwarding alias named payload that calls
the existing into_payload to preserve backward compatibility: in the impl for
FragmentParts (the type that defines into_payload), add a method pub fn
payload(self) -> Vec<u8> annotated with #[deprecated(...)] that simply forwards
to self.into_payload(); ensure the deprecation note explains it was renamed to
into_payload.
Move preamble tests into grouped modules under tests/preamble/\nwith shared helpers to keep file sizes under the limit.\n\nAdd a Makefile typecheck target to match the commit gates.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@tests/preamble/responses.rs`:
- Around line 25-89: Add the #[expect(clippy::panic_in_result_fn, reason =
"asserts provide clearer diagnostics in tests")] attribute to both test
functions success_callback_can_write_response and
failure_callback_can_write_response so the assert_eq! usage in these
TestResult-returning tests doesn't fail CI under -D warnings; place the
attribute above each async test (above #[tokio::test]) for both functions.
In `@tests/preamble/support.rs`:
- Around line 97-148: Add Rustdoc comments for the test helper items to match
existing style: document OtherPreamble, Holder, channel_holder, take_sender_io,
recv_within, success_cb, and failure_cb with /// comments that briefly describe
each item's purpose, parameters (e.g., duration, rx, holder), return values
(e.g., (Holder, oneshot::Receiver), io::Result, TestResult, callback signature),
and any important behavior (lock poisoning handling in take_sender_io, timeout
semantics in recv_within, and that callbacks send on the oneshot). Keep comments
concise and consistent with the surrounding documented helpers
(server_with_handlers, with_running_server).
In `@tests/preamble/timeouts.rs`:
- Around line 19-129: Add the Clippy expectation attribute to both test
functions preamble_timeout_invokes_failure_handler_and_closes_connection and
preamble_timeout_allows_timely_preamble by applying
#[expect(clippy::panic_in_result_fn, reason = "test uses assertions/panics
inside a Result-returning async test; panics are intentional for test
failures")] directly above each async test declaration; this narrowly silences
the lint instead of using a global allow and keeps the reason clear.
Add a before/after snippet showing the payload accessor rename\nfor PacketParts and FragmentParts in the migration guide.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/v0-1-0-to-v0-2-0-migration-guide.md`:
- Around line 3-4: The opening sentence "This guide summarizes the breaking
changes you need to address when migrating from wireframe v0.1.0 to v0.2.0."
uses second-person phrasing; change it to a neutral construction such as
replacing "you need to address" with "to address" or "that must be addressed" so
the sentence reads neutrally (e.g., "This guide summarizes the breaking changes
to address when migrating from wireframe v0.1.0 to v0.2.0."). Update the
sentence in the document's opening paragraph accordingly.
Address review feedback by clarifying the migration guide intro and\nkeeping payload-accessor documentation neutral.\n\nRefactor preamble tests to avoid excessive nesting and add helper\nRustdoc examples for shared test utilities.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@docs/v0-1-0-to-v0-2-0-migration-guide.md`:
- Around line 24-27: Update the migration guide wording to reflect that
PacketParts::payload(self) and FragmentParts::payload(self) are deprecated
aliases (not removed): replace "has been removed" with "has been deprecated" and
add a short note that these deprecated payload() methods still exist for
backward compatibility, forward to into_payload(self), and emit deprecation
warnings during the transition period.
In `@tests/preamble/responses.rs`:
- Around line 45-50: The test `success_callback_can_write_response` currently
calls `stream.read_exact(&mut buf).await?` without a timeout which can hang CI;
wrap the read in Tokio's `timeout(Duration::from_secs(1), ...)` like the
`failure_callback_can_write_response` test does so the read uses
`timeout(Duration::from_secs(1), stream.read_exact(&mut buf)).await??` (or
equivalent handling), keeping `with_running_server`, `TcpStream::connect`, and
`read_exact` usage but ensuring the timeout result is properly awaited and
errors propagated.
In `@tests/preamble/timeouts.rs`:
- Around line 133-135: The read on the success path currently uses
stream.read_exact(&mut buf).await without a deadline, which can hang CI; wrap
that await in a tokio timeout similar to the earlier pattern (e.g., use
tokio::time::timeout with the same Duration used at line 83) and handle the
timeout error before sending via result_tx.send((buf, failure_fired)); ensure
you reference the same buffer (buf), stream, and result_tx variables and
propagate/convert the timeout into the test error path instead of awaiting
indefinitely.
- Around line 85-96: Extract the four-branch match into a predicate helper that
returns TestResult<bool>: define a function like fn
is_connection_closed(read_res: std::io::Result<usize>) -> TestResult<bool> and
move the match there (Ok(0) -> Ok(true), Ok(n) ->
Err(io::Error::other(...)).into(), Err(e) if e.kind() == ConnectionReset ->
Ok(true), Err(e) -> Err(e.into())). In the original code replace the match on
read? with calling is_connection_closed(read?) and propagate the TestResult (use
?), keeping the timeout/stream.read(&mut eof) call unchanged; reference the
symbols stream.read, read, and the new is_connection_closed helper.
This was referenced Feb 5, 2026
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
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
Rename payload() to into_payload() across the codebase to better reflect ownership and consumption semantics. A deprecated payload() alias is kept to preserve compatibility while guiding users toward the new API.
Changes
Tests
Migration notes
Validation plan
◳ Generated by DevBoxer ◰
ℹ️ Tag @devboxerhub to ask questions and address PR feedback
📎 Task: https://www.devboxer.com/task/05d5a0ca-8547-4d9a-986d-b808005e7134
📝 Closes #305
Summary by Sourcery
Rename packet and fragment payload accessors to an owning into_payload() API while preserving a deprecated payload() alias for backward compatibility.
Enhancements:
Tests: