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

Replaced setNick by useNick #16

Merged
merged 2 commits into from
May 31, 2014
Merged

Replaced setNick by useNick #16

merged 2 commits into from
May 31, 2014

Conversation

sm-Fifteen
Copy link
Contributor

Replaced the setNick('NewName') function by useNick('newName", function) to ensure that the received nick is only kept for the duration of the task it needs to do.

var originalNick = core.nickname;
this.send('nick', tmpNick);
core.nickname = tmpNick;
task();
Copy link
Contributor

Choose a reason for hiding this comment

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

This only supports synchronous tasks.

@alxgnon
Copy link
Contributor

alxgnon commented May 30, 2014

If you want to support async tasks, you're going to have to implement a done style callback, otherwise, just leave useNick around. Also, if you add a method to core.irc, make sure you document it.

P.S. please run grunt, I can already see some places where the style checker will complain :P

@sm-Fifteen
Copy link
Contributor Author

I reinstated the setNick command, then. I'm not expecting either of these commands to get much use, even less in async, but here you go. I've also changed a few line to matc the code style and documented useNick(), as per alxgnon's request.

alxgnon added a commit that referenced this pull request May 31, 2014
Replaced setNick by useNick
@alxgnon alxgnon merged commit c29ff5d into master May 31, 2014
@alxgnon alxgnon deleted the newNick branch May 31, 2014 20:51
@alxgnon alxgnon added the core label Jun 18, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants