Refactor numeric metadata formatting in tonic conversion#144
Conversation
RAprogramm
deleted the
eye-of-ra/refactor-metadata_value_to_ascii-for-performance
branch
![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.
ℹ️ 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
| fn metadata_value_to_ascii<'a>( | ||
| value: &FieldValue, | ||
| formatter: &'a mut MetadataValueFormatter | ||
| ) -> Option<MetadataAscii<'a>> { | ||
| match value { | ||
| FieldValue::Str(value) => { | ||
| let text = value.as_ref(); | ||
| is_ascii_metadata_value(text).then_some(Cow::Borrowed(text)) | ||
| is_ascii_metadata_value(text).then_some(MetadataAscii::Static(text)) |
There was a problem hiding this comment.
[P0] Accepting owned strings no longer compiles
The new MetadataAscii::Static arm assumes every FieldValue::Str is backed by a &'static str. However, FieldValue::Str stores a Cow<'static, str> and callers frequently pass owned Strings via field::str(...). For those owned cases value.as_ref() only yields a borrow tied to the field, and cannot be coerced to 'static, so this branch fails to compile once an owned string is encountered. Previously we returned a borrowed Cow<'_, str> which worked for both borrowed and owned strings. Please use an owned/buffer variant for dynamic strings instead of Static to keep the API usable.
Useful? React with 👍 / 👎.
1 participant
Summary
insert_asciiTesting
https://chatgpt.com/codex/tasks/task_e_68d76c33757c832ba3c81e572d1bea8c