proto: make InternalMessageInfo functional #1129
Merged
Conversation
This file contains 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
proto/deprecated.go
Outdated
| func (*InternalMessageInfo) Merge(Message, Message) { panic("not implemented") } | ||
| func (*InternalMessageInfo) Size(Message) int { panic("not implemented") } | ||
| func (*InternalMessageInfo) Unmarshal(Message, []byte) error { panic("not implemented") } | ||
| // Deprecated: Do not use. |
There was a problem hiding this comment.
I suggest adding more detailed info about the deprecation. Include such as:
- why this would be deprecated?
- when this would be removed from codebase?
- how to migrate if the user happens using this?
There was a problem hiding this comment.
I'd prefer not. I can mention that it's an internal type, but users really should not be looking at these.
The InternalMessageInfo type only exists to implement the XXX methods on generated messages where those methods were only ever intended to be called by this module itself. Since v1.4.0, this module no longer relies on the XXX methods, so the InternalMessageInfo and its implementation is supposed to be dead code. Unfortunately, there are external usages that violate our compatibility agreement and either directly call the XXX methods or indirectly call it because some library type-asserts to the existence of these methods. This change adds minimal support for InternalMessageInfo by just calling out directly to the v2 implementation.
This was referenced Mar 16, 2021
Closed
This was referenced Mar 18, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The InternalMessageInfo type only exists to implement the
XXX methods on generated messages where those methods were
only ever intended to be called by this module itself.
Since v1.4.0, this module no longer relies on the XXX methods,
so the InternalMessageInfo and its implementation is supposed
to be dead code. Unfortunately, there are external usages that
violate our compatibility agreement and either directly call
the XXX methods or indirectly call it because some library
type-asserts to the existence of these methods.
This change adds minimal support for InternalMessageInfo by
just calling out directly to the v2 implementation.