feat: Add Status with Details Constructor#308
Conversation
|
Where should I add the tests? Wasn't quite sure where to place them as they require a generated proto fixture. |
LucioFranco
left a comment
LucioFranco
left a comment
There was a problem hiding this comment.
Looks like a great start, left one comment once that is addressed I think we can merge!
| } | ||
|
|
||
| /// Create a new `Status` with the associated code, message, and protobuf details field. | ||
| pub fn with_details(code: Code, message: impl Into<String>, details: impl Message) -> Status { |
There was a problem hiding this comment.
I actually believe we should not include a prost based one here. I think we can just have the one above that takes a Bytes and we remove the impl Into. We can also rename the one above to with_details. then let the user choose if they want to encode via prost or something else.
There was a problem hiding this comment.
Agreed. Just copy pasta from the PR I inherited. I'm fairly new to Rust so I appreciate your patience. Is this cargo check --no-default-features CI check to enforce that there's no unnecessary coupling of external crates to the core library?
LucioFranco
changed the title
2 participants
Changes are just a Copy pasta of this pr in tower-rs.
Motivation
Reference #292 for motivation.
Solution
The prescribed solution exposes two constructors with
Status::with_detailsandStatus::with_raw_detailsthat allow the caller to construct a Status with aCode,String, andBytesor aMessageto be converted into bytes.