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
fix(pubsublite): rename publish.Metadata to pscompat.MessageMetadata #3672
Conversation
A If we need this in a separate package (I think we do?), consider the name |
c4ee1d1
to
bc9a05b
Compare
bc9a05b
to
a93eeca
Compare
@tbpg Thanks for your offline suggestion about having the Parse function just return the fields, and removing the subpackage entirely. Discussed with other folks and we generally prefer parity and consistency with the other languages. This feature was requested by a user for the Java lib and we expect a small, but non-trivial number of users to need it. So I renamed the package and struct to message.Metadata. There is another option - move the struct to |
I don't understand how not having the type or extra package makes the Go library not have parity with the other languages. What is the benefit of a type over returning the fields directly? The GoDoc will document exactly what is returned. If it's in another package, it will be harder to find, there is another package for users to have to import, etc. Ideally, the function would show up right next to the thing users will need to use it with. |
We wanted a struct mostly for consistency with the other languages. Extensibility if we encode more fields (e.g. we may need to add an epoch to the partition number for repartitioning).
Good point. Maybe The |
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.
Thanks! Looks good to me.
I left one question about duplicating vs. aliasing. Can see it both ways.
Since
publish.Metadata
now also applies to message ids, it was renamed toMessageMetadata
. It is duplicated in theinternal/wire
andpscompat
packages.Follow up for #3662.
See also the Java equivalent googleapis/java-pubsublite#482.