Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't clear subscribers if channel is already private #291

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Don't clear subscribers if channel is already private #291

wants to merge 1 commit into from

Conversation

Rikkitp
Copy link

@Rikkitp Rikkitp commented Dec 17, 2014

It fixes problem when you can't use private channels at all.
See #247 (comment) for details.

@rusikf
Copy link

rusikf commented Mar 31, 2015

I think this pr must be closed, i added description to the problem here
Short: Add to websocket-rails config

  config.keep_subscribers_when_private = true

@Envek
Copy link

Envek commented Mar 31, 2015

No, IMHO it should be kept and merged. It covers the case when channel is already private and it should not clear existing subscribers when new one connects. Now it removes subscribers even if channel is already private and this option is just a workaround (config.keep_subscribers_when_private covers case when public channel becomes private).

Also I can see that this PR was merged in @g3d's fork here: g3d@52bc107 . @g3d may be you have any comments on this?

@g3d
Copy link

g3d commented Apr 2, 2015

@Envek looks like the maintainers for that repo are busy, so I created own fork and merged some PRs. It works fone for us & as you can see, travis build are green (not true for ruby 2.2.x branch) for fork.

@rusikf
Copy link

rusikf commented Apr 2, 2015

@g3d Some of the contributors, like @moaa , also created another fork, which will be merged in future, as i understood.
@moaa , @DanKnox what you think about this pull request ?

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.

None yet

4 participants