My review

[brunch]

My review

[paulmillr]

Overall, it's freaking AWESOME!

[tdd]

Hey Paul,

• Glou: that's from my original article, and was a nod to the two creators, that I work with now and then. They run the Node.js Paris meetup and mentored at the first NodeSchool Paris. Glou's a pretty good solution, but still entirely code-based (not CoC at all). You're right, it doesn't really bear mention in the "official" guide. I'll strip these.
• Link to Node/npm: yup
• Skeleton standard names: well, the guide de-emphasizes skeletons, and IIRC @es128 was hinting at their being in for a major approach change soon (more generator-like), so this should probably wait?
• Link to docs chapter: I linked to it at the beginning ("this page is your friend"), and I don't think each convention has its own anchor in the docs file, have they?
• Reviewing watcher caveats once 1.8 is out: for sure!
• More info on getting notifications set up: good point, will do.
• brunch-server.coffee: good idea. Won't mention in the docs right now as it isn't the case in 1.7.20, but will update as we go (as for the entire guide, anyway)
• Bower install: is there a less cryptic syntax to achieve the same result? Or should I just explain the syntax before or after the CLI?
• Regex preventing autoreload: not sure what the issue is here. What auto-reload? And why won't it get included? From my outsider perspective, my own regexes (my paths + Bower) overflowing to plugin inclusion sounds more like a bug than a regular behavior…?
• Jade compiler slow: oh, that's the reason? You clocked it on my example? I never would have guessed. What about CSS: the LESS preprocessor for my source Bootstrap 3 is fast enough? 👍 for adding a mention of upcoming speedups though.
• Plugin order matters: definitely. Will do.

I'm on a 3-day holiday weekend in Burgundy starting this noon, so I'll be less reactive than usual, but this will get updated and pushed before Sunday evening I guess, so Elan can come in and give it a final glance when he's back 😄

Thanks for the review!

[paulmillr]

so this should probably wait?

Yes.

Bower install

Let's just describe what the command will do.

Regex preventing autoreload

If the user would install auto-reload-brunch, its vendor file won't get included since it is located in node_modules, not bower_components. And the auto-reloading won't work. We don't do this in guide but the user may use the skeleton he made with the guide for future projects.

I never would have guessed

"Compiling" JS files takes approximately zero milliseconds. javascript-brunch just passes the data forward as-is.

Compiling jade, on the other hand, takes considerable amount of time. Parsing, tokenization etc. Just like any other compiler. Some of them are super slow, for example if you use imports in less / stylus — they do synchronous IO. Which is unacceptable to nodejs in my opinion. We have some tricks to speed-up imports in the future though, i'm already improved styl a bit.

Thank you!

[tdd]

Hey @paulmillr, @es128.

Alright, all reviews got taken care of. As both you guys felt 11 was too long, I caved in and split plugin writing in a new chapter, pushing conclusion to 13 (no superstitions around here, I hope 😉)

The demos repo was sync'd, too (regexes in config files, basically).

BTW, about plugin includes get subjected to joinTo regexes: I consider this counter-intuitive, and borderline buggy. Sure, this creates the issue of "but where do we put these in multiple-targets scenarios?". But this requires end-users to know of plugins' innards, and that kinda sucks, it breaks encapsulation in a big way.

We need to think of a better way to handle this. Perhaps auto-merge in the first file? Or use heuristics as to which ones embed third-party code? Or simply go with the first one and provide new files.*.pluginsJoinTo settings, that are single strings (target file paths)?

[paulmillr]

Or use heuristics as to which ones embed third-party code

I think we actually did this in the past. Plugin code had fake paths like vendor/auto-reload.js. There was some issue with that — I wonder what was it.

This issue would be less relevant when the npm support would be released. All stuff would be in node_modules just like brunch plugins.

[es128]

Uh yeah, I can't double check atm, but I think this concern about the plugin includes was already resolved - and that the regex joinTo would have actually worked fine.

[tdd]

@paulmillr can you browse the latest files with an eye towards this review, and close it if you're happy with the result?