Repository navigation
Reduce the size of TwirpErrorResponse - #358
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Example tests still reference removed fields, and read-only message and metadata accessors are missing.
Review effort: Balanced
Findings: 1
Open (2)
What changed in this PR
Reduces TwirpErrorResponse size by boxing its fields behind a private representation.
Changes:
- Updates construction, serialization, formatting, and field access for the boxed representation.
- Migrates library tests to constructors and accessors.
- Bumps
twirpto 0.12.0 and adopts Rust 2024.
| File | Description |
|---|---|
| crates/twirp/src/error.rs | Boxes error fields and updates methods and tests. |
| crates/twirp/Cargo.toml | Updates the version and Rust edition. |
| Cargo.lock | Records twirp version 0.12.0. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
jorendorff
force-pushed
the
jorendorff/error-size
branch
from
October 2, 2026 17:27
05bcfd4 to
fbb59d7
Compare
It's so big that Rust issues a warning when you return a Result containing it. An easy fix is to box up all the fields. This unfortunately breaks compatibility, so we bump the version number and make all fields private, with accessors.
jorendorff
force-pushed
the
jorendorff/error-size
branch
from
October 2, 2026 17:30
8290a47 to
519e76a
Compare
tclem
approved these changes
Oct 2, 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
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.
It's so big that Rust issues a warning when you return a Result containing it.
An easy fix is to box up all the fields. This unfortunately breaks compatibility, so we bump the version number and make all fields private, with accessors.