coffeefilter icon indicating copy to clipboard operation
coffeefilter copied to clipboard

Update index.js

Open tj opened this issue 11 years ago • 5 comments

I think it's more readable now

tj avatar Aug 17 '12 00:08 tj

+1 :+1: :shipit:

mattrobenolt avatar Aug 17 '12 00:08 mattrobenolt

Does not look like idiomatic coffeescript to me.

  1. You should use single quotes when not using interpolation.
  2. No need to use braces at lines 10-11.
  3. No need to use herecomments, because they won’t be parsed by Docco (coffeescript documentation generator)
  4. Rather than using %s, you can use string interpolation ("FILTERED #{filename}")

I think @gjohnson cares deeply about his code quality and he won’t merge the pull request without these changes.

paulmillr avatar Aug 17 '12 00:08 paulmillr

@visionmedia You forgot something on line 1:

# Generated by JavaScript

:)

adambiggs avatar Aug 17 '12 00:08 adambiggs

I also forgot to remove the unnecessary parenthesis, they're making it too readable ATM

tj avatar Aug 17 '12 00:08 tj

This made my night! lol But yeah, probably best to run it in some type of coffeescript VM or something before the merge, this VM could do all the optimizations (like removing parens and other things that assist in readable code).

gjohnson avatar Aug 17 '12 02:08 gjohnson