Cleanup BWC versions #5311
Cleanup BWC versions #5311
Conversation
Remove all checks put in place supporting legacy ES versions. Cleanup old versions no longer bwc supported Signed-off-by: Rabi Panda <adnapibar@gmail.com>
Gradle Check (Jenkins) Run Completed with:
|
Codecov Report
@@ Coverage Diff @@
## main #5311 +/- ##
============================================
- Coverage 71.03% 71.03% -0.01%
- Complexity 58129 58136 +7
============================================
Files 4704 4704
Lines 277266 277244 -22
Branches 40147 40136 -11
============================================
- Hits 196957 196940 -17
- Misses 64129 64214 +85
+ Partials 16180 16090 -90
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| @@ -342,7 +325,7 @@ private Map<Integer, List<Version>> getReleasedMajorGroupedByMinor() { | |||
|
|
|||
| public void compareToAuthoritative(List<Version> authoritativeReleasedVersions) { | |||
| Set<Version> notReallyReleased = new HashSet<>(getReleased()); | |||
| notReallyReleased.removeAll(authoritativeReleasedVersions); | |||
| authoritativeReleasedVersions.forEach(notReallyReleased::remove); | |||
There was a problem hiding this comment.
Just curious, why removeAll didn't cut?
There was a problem hiding this comment.
The performance of set.removeAll depends upon the size of the set vs the passed collection and the performance of the contains method of the passed collection. Here is the Java Doc. In this case, the method is called by passing the list of released versions from maven which is going to be bigger than the list of released versions in Versions.java
There was a problem hiding this comment.
That is correct, and the removeAll is smart enough to make the decision (according to same Javadoc), if we really care about performance here (which is always good but may be sometimes not strictly necessary), we probably should represent the List<Version> authoritativeReleasedVersions as a Set<Version> (why do we need duplicates here?) in the first place, wdyt?
There was a problem hiding this comment.
Yes, we could change the type to set but it would require some further changes. Do you have any particular concern with this refactoring?
There was a problem hiding this comment.
Not, not necessarily the concern but curiosity
Description
Remove all checks put in place supporting legacy ES versions. Cleanup old versions no longer bwc supported.
Issues Resolved
N/A
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.
Signed-off-by: Rabi Panda adnapibar@gmail.com