jlgrock / ClosureJavascriptFramework

A group of plug-ins that can be used in conjunction with maven to execute the Google Closure Compiler on JavaScript code. This Framework allows for scaling and modularity.
MIT License
16 stars 7 forks source link

[closure-compiler-maven-plugin] enable --@define compiler command line option #39

Closed lukas-vlcek closed 10 years ago

lukas-vlcek commented 10 years ago

@jlgrock do you think you can quickly look at this? Comments welcome. One thing I am not sure about is if we need to honour @define options in generateSyncLibrary method as well.

I am missing tests for now. I was trying to implement test according to http://maven.apache.org/plugin-testing/maven-plugin-testing-harness/getting-started/index.html but it did not work for me. Test was still complaining about some missing classes or methods... it would probably need quite careful setup of versions of additional test scoped maven plugin artifacts or something like that. I was unable to get it right.

Also documentation needs to be updated accordingly.

Closes #37

jlgrock commented 10 years ago

As I mentioned in the issue and comments, I refactored and corrected the code quality stuff a bit.

I was able to test by adding the following code to the ClosureJavaScriptJQueryExample project:

/**
 * @define {boolean} True if WTF is enabled.
 * This should be defined to false in release builds to ensure that WTF is not
 * compiled in at all. It flips all functions to nullFunction (or some
 * equivalent) and will allow the compiler to strip them out.
 */
WTF.ENABLED = true;

...

$(document.body).append('WTF.ENABLED=' + WTF.ENABLED);

when I added and removed the config in the file, it flipped this value in the append. So it appears to be working. Give a test with my changes on your code and I can release the new version.

jlgrock commented 10 years ago

Oh yeah, don't worry about the formal testing. The tools for Maven suck and I'd rather just integration test them at runtime. As for the documentation, I've updated the JavaDocs and will update the comments in the example as well as the wiki.

lukas-vlcek commented 10 years ago

I added bunch of tests of the Define parsing logic and fixed one small issue with it. Also I renamed one of existing tests to better reflect the purpose of it (but I made it in a new commit to make it easy for you to drop it if you are not fan of it).

jlgrock commented 10 years ago

Lol. Special thanks for catching my typos. Looks Great.

lukas-vlcek commented 10 years ago

IntelliJ IDEA caught them, not me.