Improve versioning support documentation and validate version #2214
Conversation
Marcono1234
force-pushed
the
marcono1234/versioning-documentation
branch
from
21c2c82 to
ea859e9
Compare
| public GsonBuilder setVersion(double ignoreVersionsAfter) { | ||
| excluder = excluder.withVersion(ignoreVersionsAfter); | ||
| public GsonBuilder setVersion(double version) { | ||
| if (Double.isNaN(version) || version < 0.0) { |
There was a problem hiding this comment.
Have added this < 0.0 check because negative version numbers are probably rather uncommon and this could cause collisions with the undocumented value -1.0 representing no version:
| if (annotationVersion > version) { | ||
| return false; | ||
| } | ||
| return version >= annotationVersion; |
There was a problem hiding this comment.
This should behave equivalently (unless someone uses NaN as version ...), but is hopefully a bit easier to understand.
There was a problem hiding this comment.
Indeed, the old code was surprising.
| if (annotationVersion <= version) { | ||
| return false; | ||
| } | ||
| return version < annotationVersion; |
There was a problem hiding this comment.
This should behave equivalently (unless someone uses NaN as version ...), but is hopefully a bit easier to understand.
eamonnmcmanus
left a comment
eamonnmcmanus
left a comment
There was a problem hiding this comment.
This seems reasonable. It's technically an incompatible change, since we never said version numbers could not be negative. But I think it would be pretty surprising if someone were using this with negative version numbers. (For what it's worth, there appears to be exactly one user of GsonBuilder.setVersion in Google's source code, outside Gson's own tests.)
2 participants
Resolves #1007
Resolves that issue by considering it a documentation issue, not an implementation issue. When
@Untilwas added by 6f59bc3, a corresponding test was also added which checks that the@Untilversion is exclusive. From the exclusion standpoint this behavior makes sense since you specify that a field exists until versionXin which it is removed. This also allows combinations of@Until(X)marking the old fields and@Since(X)marking the new fields in a class without overlap. If the value was inclusive it would create situations where for versionXa field is included, but for versionX.00001it is not included anymore.Though on the other hand maybe there are situations where it is desired that the value is inclusive, see StackOverflow question on which #1007 is based.