Fixing Issue 796 #797
Fixing Issue 796 #797
Conversation
| @@ -64,7 +64,9 @@ public abstract class Snapshot { | |||
| * | |||
| * @return an array of {@link HistogramBucket} if it is available or an empty array if not available | |||
| */ | |||
| public abstract HistogramBucket[] bucketValues(); | |||
| public HistogramBucket[] bucketValues() { | |||
| return null; | |||
There was a problem hiding this comment.
Shouldn't the default implementation return new HistogramBucket[0] to conform to the Javadoc?
Also, I realize that there is normally not much of a need for unit tests because the TCK covers the testing needs. But in this case would it make sense to add a unit test class that extends Snapshot but does not have its own implementation of this method? Simply trying to compile the class during a normal build would verify that the default implementation on Snapshot is in place.
| <version>5.1.2-SNAPSHOT</version> | ||
| <version>5.2.0-SNAPSHOT</version> |
There was a problem hiding this comment.
If the PR is changing the version to 5.2.0 to avoid the bnd baseline failures I was seeing in my local change (I was trying to use 5.1.2-SNAPSHOT) I understand this and the other version changes, but the change to the Snapshot abstract class is not a functional addition over 5.1.1 as true semantic versioning would imply if released as 5.2.0.
Releasing the change to Snapshot as 5.1.2 would be more intuitive to our consumers, but it seems the PR would have to add a bnd baseline exclusion for the updated method. I was not able to look into that. Perhaps @Channyboy can help here?
|
Close this one as this is just a demo on how to fix it normally. After some discussion, the group has agreed not to track the method update on the Snapshot and perform a micro release. The official fix is here |
No description provided.