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

adding colors #33

Merged
merged 1 commit into from
Oct 2, 2011
Merged

adding colors #33

merged 1 commit into from
Oct 2, 2011

Conversation

wraithan
Copy link
Contributor

@wraithan wraithan commented Oct 1, 2011

adds a colors.js that has some helpers for making colors easier.

@wraithan
Copy link
Contributor Author

wraithan commented Oct 1, 2011

Included in #34

@martynsmith
Copy link
Owner

This looks fine.

Do think it might be a little weird that we're exporting Client and color (i.e. different case)?

I suppose it doesn't matter too much, but what do you think about upper-casing to Color?

@wraithan
Copy link
Contributor Author

wraithan commented Oct 1, 2011

color wont be instantiated because it holds no state, it is just a
module of utilities. Client on the other hand holds state and must be
instantiated. This is a common idiom in JavaScript.

On Sat, Oct 1, 2011 at 4:20 PM, Martyn Smith
reply@reply.github.com
wrote:

This looks fine.

Do think it might be a little weird that we're exporting Client and color (i.e. different case)?

I suppose it doesn't matter too much, but what do you think about upper-casing to Color?

Reply to this email directly or view it on GitHub:
#33 (comment)

martynsmith added a commit that referenced this pull request Oct 2, 2011
@martynsmith martynsmith merged commit c7d3f36 into martynsmith:master Oct 2, 2011
@fent
Copy link
Contributor

fent commented Oct 13, 2011

I made a module for this a while ago
https://github.com/fent/irc-colors.js

has style support too, and chaining.

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.

5 participants