wal-protocol: rename Record→Command and unify payload types#4710
Merged
Conversation
This was referenced May 10, 2026
Test Results 8 files + 3 8 suites +3 5m 14s ⏱️ - 1m 20s Results for commit 082ee2e. ± Comparison against base commit 55e768b. This pull request removes 43 and adds 50 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
muhamadazmy
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0da58d5dfc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Follow-up cleanup on the v2 envelope abstraction in `crates/wal-protocol`
- Collapse the `Record::Payload` associated type. The `Command` trait now
bounds `StorageEncode + StorageDecode` directly, so each command type
IS its own payload. This removes the empty marker structs
(`pub struct AnnounceLeader; impl Record for AnnounceLeader { type
Payload = ... }`) that the `record!` macro generated, and the matching
`command!` macro is reduced to a single `Sealed`/`Command` impl.
- Drop the `RecordWithKeys` trait in favor of a blanket
`impl<C: Command + HasRecordKeys> From<C> for PartialEnvelope`, so
building a partial envelope is `payload.into()` instead of
`MyRecord::partial(payload)`.
AhmedSoliman
approved these changes
May 11, 2026
Contributor
AhmedSoliman
left a comment
AhmedSoliman
left a comment
There was a problem hiding this comment.
Very neat. Thanks @muhamadazmy a lot.
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
wal-protocol: rename Record→Command and unify payload types
Follow-up cleanup on the v2 envelope abstraction in
crates/wal-protocolRecord::Payloadassociated type. TheCommandtrait nowbounds
StorageEncode + StorageDecodedirectly, so each command typeIS its own payload. This removes the empty marker structs
(
pub struct AnnounceLeader; impl Record for AnnounceLeader { type Payload = ... }) that therecord!macro generated, and the matchingcommand!macro is reduced to a singleSealed/Commandimpl.RecordWithKeystrait in favor of a blanketimpl<C: Command + HasRecordKeys> From<C> for PartialEnvelope, sobuilding a partial envelope is
payload.into()instead ofMyRecord::partial(payload).Stack created with Sapling. Best reviewed with ReviewStack.