monokrome / jaded-brunch

Jade for brunch, supporting both dynamic and static loading jade.
MIT License
23 stars 10 forks source link

Compatibility with Brunch 1.7.0. #1

Closed matthieu closed 10 years ago

matthieu commented 10 years ago

New version of brunch seems to want the result of the compilation from the callback to allow caching.

monokrome commented 10 years ago

Adding the build JS file is akin to adding built assembly object files from C code. Note that I'm not directly comparing JS as "assembly for the web", but only stating the fact that the JS is analogous in terms of being a build artifiact. It's a waste of space, because nobody is going to copy and paste my JS file when they can just npm install jaded-brunch --save. Along with that, it is redundant. NPM itself installs the JS file with my package structure, and doesn't ship the coffee. Thus, coffee is not a dependency of the project itself.

If someone won't install a project over NPM until they've read the code, they hate coffee so much that they wont read it, or they can't take the time to clone the project and build it, then they can go find another library to use. There is no reason to justify throw build artifacts in a code repository.

As the JS file is an artifact of the build process, it will only serve as a point of confusion for people. I don't want to see people sending pull requests where they've edited my JavaScript because I was silly enough to allow redundant code into my repository.

The lib directory is in .gitignore for a reason.

The pull request for the built files should be a separate pull request from the actual code changes.

Assuming that the built files are removed, is the compatibility change in 0aff68c882307ef5aa287c5de983ba9003915eb5 actually solving any problems?

Please refer to this issue in order to understand better what is expected in the case of static files: brunch/brunch#557

This line should take care of the issue of brunch storing useless files as described: https://github.com/monokrome/jaded-brunch/blob/master/src/index.coffee#L115

matthieu commented 10 years ago

My bad, I was running on a slightly outdated version of Brunch (just a few commits before the 1.7 release, enough to miss your fix). Sorry about the confusion.

monokrome commented 10 years ago

No problem. Thanks for the pull.