Skip to content

Conversation

@upthewaterspout
Copy link
Contributor

This protocol change actually happened in 1.13.2. 1.13.1 is still using the
same protocol ordinal as 1.13.0, which is 120.

Evidence

  • b2d2054 is the commit that introduced this version in support/1.13
  • b2d2054 is only contained in rel/v1.13.2 and above
  • Running geode 1.13.1, it reports it's protocol version as 120.

(cherry picked from commit 2a90a58)

This protocol change actually happened in 1.13.2. 1.13.1 is still using the
same protocol ordinal as 1.13.0, which is 120.

Evidence
* b2d2054 is the commit that introduced this version in support/1.13
* b2d2054 is only contained in rel/v1.13.2 and above
* Running geode 1.13.1, it reports it's protocol version as 120.

(cherry picked from commit 2a90a58)
Copy link
Contributor

@jake-at-work jake-at-work left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See concerns on similar PR #6730

@upthewaterspout upthewaterspout changed the title Changing the version with ordinal 121 to be 1.13.2 GEODE-9480: Changing the version with ordinal 121 to be 1.13.2 Jul 30, 2021
Copy link
Contributor

@onichols-pivotal onichols-pivotal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as it's clear that this is a cosmetic change, not intended to alter behavior in any way, I'm ok with it. Just adding a comment might have been equally effective, and probably still a good idea. Many developers get confused because there is a vestigial version number attached to serialization ordinals that may look similar to Geode product version (which is a totally unrelated concept). However, if you want to follow the convention that constant names for serialization ordinals should match the product version they were introduced (rather than simply sequential numbering), that's cool. Just comment that this is a nice convention to make it easier to reason about what versions introduced serialization changes.

@upthewaterspout upthewaterspout merged commit 4af660d into apache:support/1.14 Aug 11, 2021
@upthewaterspout upthewaterspout deleted the feature/GEODE-9480-1.14 branch August 11, 2021 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants