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

Added SASL support #125

Merged
merged 4 commits into from
Sep 6, 2013
Merged

Added SASL support #125

merged 4 commits into from
Sep 6, 2013

Conversation

gsf
Copy link
Contributor

@gsf gsf commented Dec 27, 2012

Set sasl: true in options, along with nick, userName, and password,
for SASL login (required for some IP ranges for connecting to
Freenode, etc.).

No docs or tests, sorry. Happy to provide if you give some guidance.

Gabriel Farrell added 2 commits December 27, 2012 14:39
Set sasl: true in options, along with nick, userName, and password,
for SASL login (required for some IP ranges for connecting to
Freenode, etc.).
@quentinxs
Copy link
Contributor

While I applaud your effort, you may want to rework your code a bit as CAP is/ can be used for more than SASL. Other than that, documentation is as easy as updating docs/API.rst to reflect your changes to the library.

@gsf
Copy link
Contributor Author

gsf commented Dec 28, 2012

Yes, I was kind of greedy with the CAP signals since nothing else in the library is using them yet. The third commit makes it more specific. When a use case comes up for other CAP functionality it can be accommodated then.

The last commit adds the option to the docs.

If these additions don't hinder other uses, I would really appreciate their inclusion. I can't connect to Freenode via my phone without them (anything on the T-Mobile network is subject to SASL).

@gsf
Copy link
Contributor Author

gsf commented Dec 28, 2012

I could also move those listeners into the if ( self.opt.sasl ) block so that they only exist when SASL is enabled.

@kamaln7
Copy link

kamaln7 commented Aug 8, 2013

👍

martynsmith added a commit that referenced this pull request Sep 6, 2013
@martynsmith martynsmith merged commit bafa696 into martynsmith:master Sep 6, 2013
@martynsmith
Copy link
Owner

This looks fine, I notice that we get fairly wildly varying coding styles, I think I might get some jshint config set up and try to get everyone to conform to that :-)

@gsf
Copy link
Contributor Author

gsf commented Sep 14, 2013

Thanks for merging this in!

@gsf gsf deleted the sasl branch September 14, 2013 01:07
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.

4 participants