Ported simple.js to coffeescript #28
Conversation
jvelliott
force-pushed
the
coffeescript-example
branch
3 times, most recently
from
f201458 to
e103330
Compare
|
Your coffeescript "ternary" is generally right in that you're using an if/else as an expression. The maybe "nicer" way would be to one line it: I'll leave a couple other comments on your commit with some more coffeescript tips (not that anything you did was wrong). |
| channel = slack.getChannelGroupOrDMByID(message.channel) | ||
| user = slack.getUserByID(message.user) | ||
| time = message.ts | ||
| text = message.text |
There was a problem hiding this comment.
There's a nice desctructuring syntax in cs: {type, ts, text} = message
|
Thanks for the feedback. I'll update it over the weekend. Do you prefer I make a separate commit, or rebase onto this one? |
Whatever you like. I think it also makes sense to actually replace the JS version. Having both in the source, especially when they're not actually the "same" code, seems confusing. To do that it should be added to the |
| @@ -0,0 +1,73 @@ | |||
| #!/usr/bin/env coffee | |||
There was a problem hiding this comment.
You don't need this unless the file is meant to be executable (which it probably shouldn't be).
OK, that sounds good to me. Should I go through and comment on the new compiled version, or just leave it alone? You do actually want it committed, right? |
|
Let's not commit the compiled JS. It can be compiled by the same grunt task that handles the rest of the library. So the JS file should also be removed from Git and gitignored. Let me know if any of that doesn't make sense or if you want any help. |
| cwd: 'examples' | ||
| src: ['*.coffee'] | ||
| dest: 'examples' | ||
| ext: '.js' |
There was a problem hiding this comment.
Hope this looks good. Never worked with grunt before!
|
Not sure if this belongs here, but there was something I was going to poke at the sample code about. There are a variety of things that will crash the javascript sample, like deleting a message. Some 'message' subtypes have no user or text associated with them so the console.log throws. |
|
Hah, it's a fair point. Guess I never noticed that deleting/editing/a bunch of other stuff all hits the on "message" callback. Seems to me like either the project or the APIs are being pretty overbroad with those things, but it can't hurt to do a bit of null checking. After some experimenting.. it looks like message.subtype is not set when it's a normal message, but that doesn't feel like a safe condition to reason on. |
|
@jvelliott is this all ready to merge? |
|
As far as I know! |
jvelliott
force-pushed
the
coffeescript-example
branch
from
e223ead to
68d6b1a
Compare
Better explanation on how to run simple_reverse.coffee example Added examples to the Gruntfile Responded to issues from code review.
New, better variable names in comprehensions.
…r channel cannot be determined.
jvelliott
force-pushed
the
coffeescript-example
branch
from
68d6b1a to
03ad8a7
Compare
|
Rebased commits so they all show up together after merge. |
Ported simple.js to coffeescript
|
Thanks! |
adds SlackEventAdapter.stop() method
Fix callback handling, add options
Tried to do a direct conversion to help any other noobs out there trying to learn coffeescript.
A couple list comprehensions. Not sure if there's a nicer way to do ternary in coffeescript?