Skip to content

Conversation

@PhilipDukhov
Copy link
Contributor

Allows authenticating for private channels. Fixes #52

@FZambia
Copy link
Member

FZambia commented Feb 11, 2023

@PhilipDukhov many thanks! That's an unfortunate miss..

Could you please put options before listener in the constructor? To match order of arguments in a Client constructor.

@PhilipDukhov
Copy link
Contributor Author

@FZambia sure, done 👍

@PhilipDukhov
Copy link
Contributor Author

Passed the checks

@FZambia
Copy link
Member

FZambia commented Feb 11, 2023

Since we keep the old API here, then what is it better in your opinion:

  1. Match Client constructor and have SubscriptionOptions a second argument in new API?
  2. Or, have SubscriptionOptions a third argument in new API (after listener, like in your original PR version)?

For me 2 seems more natural (more IDE friendly as the beginning of two methods match), what do you think?

@PhilipDukhov
Copy link
Contributor Author

I'd prefer using same style in whole project, so I'd prefer 1.

Also even if we could make breaking changes, I think options on the second place still looks more natural - if you create listener on place, it's easier to read with listener as the last argument, e.g.

client.newSubscription(
        "chat:index",
        new SubscriptionOptions(),
        new SubscriptionEventListener() {
            @Override
            public void onSubscribing(Subscription sub, SubscribingEvent event) {
                MainActivity.this.runOnUiThread(() -> tv.setText(String.format("Subscribing to %s, reason: %s", sub.getChannel(), event.getReason())));
            }
        }
);

@FZambia FZambia merged commit 07370a5 into centrifugal:master Feb 11, 2023
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.

How to subscribe to private channel in v2 API?

2 participants