Skip to content

Allow different types on with_meta arguments#301

Merged
itsibitzi merged 1 commit intomainfrom
itsibitzi/20260213-nicer-types-with-meta
Feb 13, 2026
Merged

Allow different types on with_meta arguments#301
itsibitzi merged 1 commit intomainfrom
itsibitzi/20260213-nicer-types-with-meta

Conversation

@itsibitzi
Copy link
Copy Markdown
Contributor

Before this change we constrained both the key and the value on with_meta to be the same type. This resulted in stuff like blah.with_meta("key", &value.to_string()), which is basically turning value from a T: ToString into a String into a &str and then inside the function back to a String again. Twice the allocation, twice the fun.

This PR ruins that fun by allowing the key and value to be different kinds of ToString.

This could be more idiomatic as a pair of impl Into<String> since that allows things like an owned string to not have to reallocate, but that would be a breaking change so this will perhaps do for now. Though ToString does allow anything with Display to also be used here which is maybe nice?

@itsibitzi itsibitzi requested a review from a team as a code owner February 13, 2026 02:26
Copilot AI review requested due to automatic review settings February 13, 2026 02:26
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes an unnecessary type constraint in the with_meta method that previously required both key and value parameters to be the same type. The change eliminates redundant string conversions where callers had to convert values to strings just to satisfy the single generic type parameter.

Changes:

  • Updated with_meta signature to accept two different ToString types instead of one
  • Simplified call sites by removing unnecessary .to_string() conversions

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
crates/twirp/src/error.rs Modified with_meta method signature to accept separate generic types for key and value
crates/twirp/src/server.rs Removed redundant .to_string() calls when passing error values to with_meta

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Member

@tclem tclem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Good fix.

@itsibitzi itsibitzi added this pull request to the merge queue Feb 13, 2026
Merged via the queue into main with commit 5684ad3 Feb 13, 2026
10 of 11 checks passed
@itsibitzi itsibitzi deleted the itsibitzi/20260213-nicer-types-with-meta branch February 13, 2026 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants