pivotal-jbarrett commented on a change in pull request #6702:
URL: https://github.com/apache/geode/pull/6702#discussion_r680259788
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/EventID.java
##########
@@ -87,7 +87,7 @@ public void incBreadcrumbCounter() {
/** The versions in which this message was modified */
@Immutable
- private static final KnownVersion[] dsfidVersions = new KnownVersion[]
{KnownVersion.GFE_80};
+ private static final KnownVersion[] dsfidVersions = new KnownVersion[0];
Review comment:
eh... I went this route for two reasons.
1) We have been trending away from `null` to things like
`Collections.emptyList()` so this `Object[0]` is effectively the same.
2) It feels strange having something that says it supports versions, by the
existence of the interface, but return `null` when asked. An empty array/list
felt better as a way to say, yes it is supported but there are no versions yet.
The larger change might be to remove the interface entirely from these
classes but I wanted to keep the changes smaller.
On the other hand your argument for consistency is good. I would need to
look. I don't recall seeing others that returned `null`, just that it
defensively checked for null.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
