-
Notifications
You must be signed in to change notification settings - Fork 527
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: TypeUrl
trait + Any
encoding support
#858
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
//! Support for associating a type URL with a [`Message`]. | ||
|
||
use crate::Message; | ||
|
||
/// Associate a type URL with the given [`Message]`. | ||
pub trait TypeUrl: Message { | ||
/// Type URL for this [`Message`]. They take the form: | ||
/// | ||
/// ```text | ||
/// /<package>.<TypeName> | ||
/// ``` | ||
/// | ||
/// For example: | ||
/// | ||
/// ```text | ||
/// /foo.bar.baz.MyTypeName | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think the slash is part of the type URL, the type name is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, I guess this particular encoding is an artifact of the type URLs I'm working with and not the standard encoding. I'm still unclear though if all type URLs are expected to be prefixed with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is difference between type url and full type name of a protobuf message. For instance the full type name is not expected to have slashes. The Type URL in a So if this Type URL mentioned here is supposed to end up in an any then by specification, it should contain at least one slash. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this TYPE URL we are talking about in this PR is supposed to be used to compose the TYPE URL of an any then we shouldn't be talking about TYPE URL but we should be talking about FULLNAME of a protobuf message, they're different concepts. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, the type url in Any should have a slash, but the type name that we attach to the trait should just be the full type name without a slash. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Then this trait should not be called And the way we match a type URL is not the correct one: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The use case is populating the If you're saying the type URL can always be computed from the fullname by prepending a slash, that's fine. That's what the implementation I'm using is expecting. |
||
/// ``` | ||
const TYPE_URL: &'static str; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just went with a In my mind the goal is to get |
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these might benefit from a shorter
from_msg
/to_msg
naming, but I went with existing precedent which seems to spell out the whole wordmessage
.