Repository navigation
Conversation
waynercheung
left a comment
There was a problem hiding this comment.
[SHOULD] Please record the pre-change serialization comparison required by #7013, including the commands, fixtures and results, or add fixed regression fixtures for Endpoint, HelloMessage and BackupMessage. The current PR does not document this comparison.
[DISCUSS] Please reconcile #7013's conditional deprecation requirement with the clarification in #6921 that these are unsupported legacy types. Record the agreed classification and describe the generated-type removal and expected descriptor change in this PR's compatibility notes.
The four removed legacy message types have been unused by java-tron for years and are not supported external APIs, as clarified in #6921. Their removal therefore does not require a deprecation cycle. The removal of the generated types and the corresponding descriptor changes are intentional and can be documented in the PR’s compatibility notes. |
|
Thanks. On the serialization comparison: agreed, no additional permanent regression tests are needed for this cleanup. For the record, I ran the comparison against the merge base 6d5adc4: [SHOULD] On the classification: agreed. Please add the compatibility note to the PR description before merge, and update the last paragraph of #7013 to match. Suggested text:
[NIT] Please correct "defintion" to "definition" in the PR title. |
What does this PR do?
close #7013.
PingMessage,PongMessage,FindNeighbours, andNeighboursdefinitions fromprotocol/src/main/protos/core/Discover.proto.Endpoint, referenced byHelloMessage.from, andBackupMessage, used by backup keepalive messages, with their existing fields and options.p2p/src/main/proto/Discover.protoand align the UDP discovery examples and field descriptions with the implementation, including IPv6 addresses, network identifiers, and response timestamps.Why are these changes required?
These definitions are redundant leftovers from the libp2p split and are no longer referenced by java-tron's runtime code. UDP discovery uses the separate p2p definitions, brought into this repository in #6992. Removing the legacy definitions reduces maintenance overhead and avoids confusion about the active discovery protocol.
Part of the cleanup tracked in #6921.