Remove previously deprecated methods and types #2277
Conversation
| * @param onComplete callback on completion signal | ||
| * @param initialContext {@link Context} for the rails | ||
| */ | ||
| @Deprecated |
There was a problem hiding this comment.
actually, I don't think this one should be removed :( It looks like it was deprecated by mistake, because ultimately the one we'd want removed is the one with a Consumer<Subscription> (see the TODO on subscribe variant with such a Consumer below)
There was a problem hiding this comment.
please revert the removal of that method, and delete the @Deprecated instead 🙇
| @@ -41,19 +42,13 @@ public void sourceNull() { | |||
| new FluxRetryPredicate<>(null, e -> true); | |||
There was a problem hiding this comment.
The implementation FluxRetryPredicate can most likely also be deleted. Please double check remaining relevant tests are either kept in this test class or moved to FluxRetryWhenTest as a @Nested test maybe?
| @@ -34,14 +36,14 @@ public void twoRetryNormalSupplier() { | |||
|
|
|||
| StepVerifier.create(Mono.fromCallable(i::incrementAndGet) | |||
There was a problem hiding this comment.
same as for FluxRetryPredicate
| @@ -1021,28 +1021,6 @@ default Duration verifyTimeout(Duration duration) { | |||
| */ | |||
| Step<T> expectNoFusionSupport(); | |||
|
|
|||
| /** | |||
There was a problem hiding this comment.
there was no "to be removed" even though this one has been deprecated since 3.3.1... I wonder if there could be anybody out there testing that no onSubscribe happens for a given duration (which would be a legitimate-ish verification). Can restore it for now and open an issue for that specific case to remove the method and add a suitable alternative in a follow-up PR ?
There was a problem hiding this comment.
But no onSubscribe after subscribe() would be a spec violation, isn't it?
There was a problem hiding this comment.
yes, but a delayed onSubscribe wouldn't.
There was a problem hiding this comment.
There was a problem hiding this comment.
right... used to be a gray area. let's remove it then, even though the @Deprecated was intended as a deterrent to using that method without at least thinking about calling expectSubscription().
There was a problem hiding this comment.
Ok, will re-remove it :D 👍
| @@ -34,14 +36,14 @@ public void twoRetryNormalSupplier() { | |||
|
|
|||
| StepVerifier.create(Mono.fromCallable(i::incrementAndGet) | |||
| .doOnNext(v -> { | |||
| if(v < 4) { | |||
| if (v < 4) { | |||
There was a problem hiding this comment.
the tests of removed retry operators should either be moved to xxxRetryWhenTest or there should be a comment in that test class pointing to xxxRetryPredicateTest. also there are SuppressWarnings("deprecation") in these tests that are not necessary anymore
build.gradle
Outdated
| @@ -28,7 +28,7 @@ buildscript { | |||
| plugins { | |||
| id "com.github.johnrengelman.shadow" version "4.0.2" | |||
| id 'org.asciidoctor.convert' version '1.5.11' | |||
| id "me.champeau.gradle.japicmp" version "0.2.6" | |||
| id "me.champeau.gradle.japicmp" version "0.2.8" | |||
There was a problem hiding this comment.
IIRC we're stuck in 0.2.6 because newest japicmp by default reject introducing new default method()s (which we need in 3.4), and the gradle plugin doesn't let us disable that feature.
did you have a strong need for 0.2.8 ?
There was a problem hiding this comment.
Ah, okay. It worked fine so I proceeded. Will revert
Codecov Report
@@ Coverage Diff @@
## master #2277 +/- ##
============================================
+ Coverage 82.95% 84.39% +1.44%
- Complexity 4509 5226 +717
============================================
Files 381 377 -4
Lines 31766 32827 +1061
Branches 6071 6227 +156
============================================
+ Hits 26351 27705 +1354
+ Misses 3791 3487 -304
- Partials 1624 1635 +11
Continue to review full report at Codecov.
|
No description provided.