search

scalableminds / amd-optimize

An AMD (RequireJS) optimizer that's stream-friendly. Made for gulp.
MIT License
162 stars 29 forks source link

Better implementation of attaching comments #29

Closed kirill-konshin closed 9 years ago

kirill-konshin commented 9 years ago

In new implementation comments are applied to AST immediately after it was created, this makes traversing of AST safer. Tests are feeling good plus I have placed extra assertions.

normanrz commented 9 years ago

Thanks again. Looks like a much better implementation. I noticed you ran amdOptimize twice in the test (https://github.com/scalableminds/amd-optimize/pull/29/files#diff-f060e4610ac4d0fcc1b4b55d52532777R683), so I removed one.

kirill-konshin commented 9 years ago

Yes, this most likely was a merge issue or something... Double call is not needed obviously.

kirill-konshin commented 9 years ago

By the way, overall very good coding style, was a pleasure to make contribution. Good job and thank you for nice and useful plug-in.

normanrz commented 9 years ago

Thanks :-)

joeuy commented 9 years ago

Could we support this option also on shimed modules?

kirill-konshin commented 9 years ago

What is wrong with shims? As far as I remember they were working...

joeuy commented 9 years ago

Using the following configuration, license comments got stripped from shim modules

"amdConfig": { "preserveComments": true, "shim": { "jQuery": { "exports": "jQuery", "preserveComments": true } ... }

thanks!

kirill-konshin commented 9 years ago

Can you create a separate ticket and I'll look into your issue.