[GitHub] [geode] upthewaterspout commented on a change in pull request #6729: GEODE-9480: Changing the version with ordinal 121 to be 1.13.2

[GitBox <gi...@apache.org>]

upthewaterspout commented on a change in pull request #6729:
URL: https://github.com/apache/geode/pull/6729#discussion_r680088601



##########
File path: geode-serialization/src/main/java/org/apache/geode/internal/serialization/KnownVersion.java
##########
@@ -205,12 +205,12 @@
       new KnownVersion("GEODE", "1.13.0", (byte) 1, (byte) 13, (byte) 0, (byte) 0,
           GEODE_1_13_0_ORDINAL, true);
 
-  private static final short GEODE_1_13_1_ORDINAL = 121;
+  private static final short GEODE_1_13_2_ORDINAL = 121;

Review comment:
       I'm not sure I follow your reasoning here. 1.13.1 released with ordinal 120. 1.13.2 released with ordinal 121.
   
   The problem here is that the name of this constant is wrong. It should be called GEODE_1_13_2_ORDINAL, because that is the release in which this ordinal was used.
   
   I guess there is a question of whether some external code is using this constant, since there will no longer be a constant named GEODE_1_13_1_ORDINAL. However this is an internal class, so it can be expected to change - in fact this whole class was renamed in 1.14. The most likely project to use these internal constants would be spring data geode, but that project does not use this internal class.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org