pivotal-jbarrett commented on a change in pull request #6500:
URL: https://github.com/apache/geode/pull/6500#discussion_r636963137
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/persistence/DiskInitFileParser.java
##########
@@ -567,24 +568,20 @@ private void readEndOfRecord(DataInput di) throws
IOException {
}
}
- private PersistentMemberID readPMID(CountingDataInputStream dis,
KnownVersion gfversion)
+ private PersistentMemberID readPMID(CountingDataInputStream dis)
throws IOException, ClassNotFoundException {
int len = dis.readInt();
byte[] buf = new byte[len];
dis.readFully(buf);
- return bytesToPMID(buf, gfversion);
+ return bytesToPMID(buf);
}
- private PersistentMemberID bytesToPMID(byte[] bytes, KnownVersion gfversion)
+ private PersistentMemberID bytesToPMID(byte[] bytes)
throws IOException, ClassNotFoundException {
ByteArrayInputStream bais = new ByteArrayInputStream(bytes);
DataInputStream dis = new DataInputStream(bais);
PersistentMemberID result = new PersistentMemberID();
- if (KnownVersion.GFE_70.compareTo(gfversion) > 0) {
- result._fromData662(dis);
- } else {
- InternalDataSerializer.invokeFromData(result, dis);
- }
+ InternalDataSerializer.invokeFromData(result, dis);
Review comment:
I think if this method needs versioning support added it should come
when there is a version change here or a separate PR to add versioning support
and tests. The only change here was to pull out the obsolete version check
which left us with the original un-versioned from data call.
Going through all these cleanups and digging deep into areas of code I
haven't been I have noticed there is inconsistent use of
`VersionedDataInputStream` and `VersionedDataSerializable`. I wonder if we
should have an effort to normalize on the versioned versions (🥁) and deprecate
the older stuff.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
