Skip to content

Conversation

maxwellsayles
Copy link
Contributor

With -Werror and -Woverloaded-virtual, we cannot use createManagedInstance with a Subscriber<T> where T != Payload.

test/simple/SubscriberNonPayloadTest.cpp will fail without the changes to MemoryMixin.h

Other mixins also refer to Payload directly, but I can fix those up in subsequent diffs.

…stance` with a `Subscriber<T>` where `T != Payload`.

test/simple/SubscriberNonPayloadTest.cpp will fail without the changes to MemoryMixin.h

Other mixins also refer to Payload directly, but I can fix those up in subsequent diffs.
/// is implementing the (virtual) methods of the
/// Subscription or the Subscriber interface.
template <typename Base>
template <typename Base, typename Payload = Payload>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand the intention of your change, shouldn't this be?

typename T = Payload

Reactive Streams isn't tied to Payload https://github.com/ReactiveSocket/reactive-streams-cpp/blob/master/include/ReactiveStreams.h

Copy link
Contributor Author

@maxwellsayles maxwellsayles Sep 2, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure typename T = Payload. The issue is on line 71 below where we have

  void onNext(Payload payload) {

I want Payload to be generic. typename T = Payload might be more explicit and then I'll change Payload elsewhere.

@lehecka lehecka merged commit 39fd881 into rsocket:master Sep 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants