eclipse / microprofile-reactive-streams-operators

Microprofile project
Apache License 2.0
80 stars 33 forks source link

Fix signature of fromSubscriber #149

Closed Azquelt closed 4 years ago

Azquelt commented 4 years ago

When creating a SubscriberBuilder<T> from a Subscriber, we need a Subscriber which can accept all objects of type T.

Therefore, we need a Subscriber which accepts either T or a supertype of T.

Therefore, we need Subscriber<? super T> rather than Subscriber<? extends T>.

This is a bugfix, but it's also a breaking change. I'm not sure if that's ok or whether we need to look at alternative non-breaking ways of fixing this, which would probably be uglier.

Fixes #134

Azquelt commented 4 years ago

Need to update the copyright headers Done

Azquelt commented 4 years ago

In the hangout, we said we needed to bump the version of the package and the spec because this is a breaking change.

Also need to make sure the baseline plugin is working so it tells us when we forget to do this...

Azquelt commented 4 years ago

The baseline plugin does not consider this to be a breaking change because it does not affect binary compatibility because the signatures after erasure are the same.

Interestingly, this appears to be the desired behaviour: bndtools/bnd#1915

The comments on that PR suggest that we would not want to increment the package version for this change, even though it would break source compatibility.

Unless someone objects, I'm going to assume that the above is true and increment the project version to 2.0 but leave the OSGi package version alone. I'll make another PR to get the baseline plugin working and define the package versions correctly.

Emily-Jiang commented 4 years ago

@Azquelt since the existing app continues to work without breaking, I think it is best not to update the version.