Add messageID from string and from parts#526
Open
addisonj wants to merge 3 commits into
Open
Conversation
If we want to deal with string message ids, we need methods for being able to create them from either a string or a using the individual parts of the message id. This exposes some constructors for this. Tests are needed for the string message parsing, but it is taking from pulsarctl where it is well used
wolfstudy
reviewed
May 24, 2021
| } | ||
| return id.equal(rmsgid) | ||
| } | ||
|
|
Member
There was a problem hiding this comment.
The MessageID is an interface, Are we here trying to compare whether two interfaces are equal?
cckellogg
reviewed
May 24, 2021
| // Serialize the message id into a sequence of bytes that can be stored somewhere else | ||
| Serialize() []byte | ||
| // String the message id represented as a string | ||
| String() string |
Contributor
There was a problem hiding this comment.
This is not needed the struct just needs to implement the Stringer interface
https://golang.org/pkg/fmt/#Stringer
| // String the message id represented as a string | ||
| String() string | ||
| // Equals indicates to message IDs are equal | ||
| Equals(other MessageID) bool |
Contributor
There was a problem hiding this comment.
I don't think we should add this either. We should avoid changing the interfaces since it's a breaking change and this can be accomplished with out doing that. We can add a util method like messageIDsEqual or MessageIDsEqual.
Member
|
ping @addisonj any update for this? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
If we want to deal with string message ids, we need methods for being
able to create them from either a string or a using the individual parts
of the message id.
This exposes some constructors for this as well as an equals method to make testing more straight forward (but I am not sure if we want to expose equals...)
This is primarily to match the java implementation, but if we don't want to add the string version, we should at least expose the method to make up a message id from the individual parts.
This commit adds tests for the needed new methods