kimroen / ember-cli-coffeescript

Adds precompilation of CoffeeScript files and all the basic generation types to the ember generate command.
MIT License
72 stars 49 forks source link

Add parameters for initializer function as comment #62

Closed ohcibi closed 9 years ago

ohcibi commented 9 years ago

I noticed that the blueprinted function had no parameters and I kept looking which of them came first. I then saw that the original one had the parameters as comments and added them here as well.

kimroen commented 9 years ago

This doesn't run, unfortunately.

I wanted to do it this way, but it doesn't look like CoffeeScript has inline comments like this. The comment above the function is the best we can do at the moment, as far as I can tell.

ohcibi commented 9 years ago

Sorry. My syntax highlighting tricked me.

kimroen commented 9 years ago

If you have ideas for making it more obvious, I'm very happy to improve it. The comment is obviously not clear enough, as you didn't see it!

ohcibi commented 9 years ago

I don't even know why the parameters aren't not just in the initialiser with out a comment? They'll hurt nobody and thats what initializers do get, when they are called... Otherwise we could just put a parameterized version into the comment above.

kimroen commented 9 years ago

Probably because in the original blueprint, jshint wil complain about the unused variable, which will break tests.

ohcibi commented 9 years ago

-.-.

Can't we just disable this specific issue in jshint?

kimroen commented 9 years ago

And that will be the case in CoffeeScript too, once we get around to registering the linter with ember-cli properly.

Well, there's a reason why it's there, don't you think? You don't want unused variables lying around!

ohcibi commented 9 years ago

Well, I didn’t knew that the linter is that strict on unused params, but I didn’t want to argue on that topic at all actually, just didn’t knew the ratio behind it.

The point of this PR was to give the developer a hint what the right order of the parameters would be. I now understand that you wanted to give this hint by telling them in the right order (even after you told me, it wasn’t obvious 8-)). Maybe we’d just add a signature like it would appear in some documentation, or just the function head with parameters?

# initialize = (container, application) ->