I think if we are making this on by default, we need to make it easy to disable. I think I would prefer it to be opt-in, at least initially, since for it to be useful other changes have to be made (their server needs to know to serve the .gz version instead) and probably those people already have the compression done in their deploy step. For others, it will be slower with no benefit until they do modify their server.
Otherwise, LGTM with a few nits (I did not look at the blacklist implementation). http://gwt-code-reviews.appspot.com/254801/diff/1/2 File user/src/com/google/gwt/precompress/Precompress.gwt.xml (right): http://gwt-code-reviews.appspot.com/254801/diff/1/2#newcode24 user/src/com/google/gwt/precompress/Precompress.gwt.xml:24: <define-linker name="precompress" class="com.google.gwt.precompress.linker.PrecompressLinker" /> Should probably include a module that does nothing but disable this (presumably by clearing the regexes?) so people that already have their own deployment setup to do this can get the original behavior. http://gwt-code-reviews.appspot.com/254801/diff/1/3 File user/src/com/google/gwt/precompress/linker/PrecompressLinker.java (right): http://gwt-code-reviews.appspot.com/254801/diff/1/3#newcode64 user/src/com/google/gwt/precompress/linker/PrecompressLinker.java:64: SortedSet<com.google.gwt.core.ext.linker.ConfigurationProperty> properties, Why should this be SortedSet instead of just Iterable, since the only thing you use from it is iteration? http://gwt-code-reviews.appspot.com/254801/diff/1/3#newcode110 user/src/com/google/gwt/precompress/linker/PrecompressLinker.java:110: context.getConfigurationProperties(), PROP_PATH_REGEXES).getValues()); I am confused by calling this a blacklist -- isn't the property value the list of extensions to compress? http://gwt-code-reviews.appspot.com/254801/diff/1/3#newcode145 user/src/com/google/gwt/precompress/linker/PrecompressLinker.java:145: byte[] buf = new byte[10000]; Should be a constant instead. http://gwt-code-reviews.appspot.com/254801/diff/1/8 File user/test/com/google/gwt/precompress/linker/PrecompressLinkerTest.java (right): http://gwt-code-reviews.appspot.com/254801/diff/1/8#newcode253 user/test/com/google/gwt/precompress/linker/PrecompressLinkerTest.java:253: artifacts.add(emit("data.xml", fooFileContents())); Should include a non-compressible file and make sure it doesn't leave the compressed version. http://gwt-code-reviews.appspot.com/254801/diff/1/8#newcode264 user/test/com/google/gwt/precompress/linker/PrecompressLinkerTest.java:264: propPathRegexes.values.add(".*\\.css"); Should also test with empty set of regexes, which is what would happen if someone needed to disable it. http://gwt-code-reviews.appspot.com/254801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors To unsubscribe from this group, send email to google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this email with the words "REMOVE ME" as the subject.
