submitted as r9893 Thanks for the review!
http://gwt-code-reviews.appspot.com/1375802/diff/10040/dev/core/src/com/google/gwt/dev/javac/PersistentUnitCache.java File dev/core/src/com/google/gwt/dev/javac/PersistentUnitCache.java (right): http://gwt-code-reviews.appspot.com/1375802/diff/10040/dev/core/src/com/google/gwt/dev/javac/PersistentUnitCache.java#newcode256 dev/core/src/com/google/gwt/dev/javac/PersistentUnitCache.java:256: shutDownLatch.await(5000, TimeUnit.MILLISECONDS); On 2011/03/24 20:24:44, scottb wrote:
Do we really need this? I'm thinking maybe we should just wait
forever. I thought it would be better to have a timeout, as I don't want to wait forever if there is some kind of filesystem hang trying to delete a file or something. In that case, the program won't shut down properly, leaving DevMode still running with a port open. http://gwt-code-reviews.appspot.com/1375802/diff/10040/dev/core/src/com/google/gwt/dev/javac/PersistentUnitCache.java#newcode318 dev/core/src/com/google/gwt/dev/javac/PersistentUnitCache.java:318: } On 2011/03/24 20:24:44, scottb wrote:
I think the other way was better, LinkedBlockingQueue never blocks on
put/add. Done. http://gwt-code-reviews.appspot.com/1375802/diff/10040/dev/core/src/com/google/gwt/dev/javac/PersistentUnitCache.java#newcode365 dev/core/src/com/google/gwt/dev/javac/PersistentUnitCache.java:365: } On 2011/03/24 20:24:44, scottb wrote:
Same in these two cases, add() is better.
Done. http://gwt-code-reviews.appspot.com/1375802/diff/10040/dev/core/src/com/google/gwt/dev/javac/PersistentUnitCache.java#newcode447 dev/core/src/com/google/gwt/dev/javac/PersistentUnitCache.java:447: } On 2011/03/24 20:24:44, scottb wrote:
Couldn't much of the above code be replaced with a call to
getCacheFiles()? Done. http://gwt-code-reviews.appspot.com/1375802/diff/10040/dev/core/src/com/google/gwt/dev/javac/UnitCache.java File dev/core/src/com/google/gwt/dev/javac/UnitCache.java (right): http://gwt-code-reviews.appspot.com/1375802/diff/10040/dev/core/src/com/google/gwt/dev/javac/UnitCache.java#newcode52 dev/core/src/com/google/gwt/dev/javac/UnitCache.java:52: void remove(CompilationUnit unit); On 2011/03/24 20:24:44, scottb wrote:
Not called from the unit test anywhere.
Done. http://gwt-code-reviews.appspot.com/1375802/diff/10040/dev/core/test/com/google/gwt/dev/javac/MemoryUnitCacheTest.java File dev/core/test/com/google/gwt/dev/javac/MemoryUnitCacheTest.java (right): http://gwt-code-reviews.appspot.com/1375802/diff/10040/dev/core/test/com/google/gwt/dev/javac/MemoryUnitCacheTest.java#newcode23 dev/core/test/com/google/gwt/dev/javac/MemoryUnitCacheTest.java:23: * Unit test for {@link MemoryUnitCache} On 2011/03/24 20:24:44, scottb wrote:
Checkstyle wants a period.
Done. http://gwt-code-reviews.appspot.com/1375802/diff/10040/dev/core/test/com/google/gwt/dev/javac/MockCompilationUnit.java File dev/core/test/com/google/gwt/dev/javac/MockCompilationUnit.java (right): http://gwt-code-reviews.appspot.com/1375802/diff/10040/dev/core/test/com/google/gwt/dev/javac/MockCompilationUnit.java#newcode25 dev/core/test/com/google/gwt/dev/javac/MockCompilationUnit.java:25: */ On 2011/03/24 20:24:44, scottb wrote:
copy header should go above imports
Done. http://gwt-code-reviews.appspot.com/1375802/diff/10040/dev/core/test/com/google/gwt/dev/javac/MockCompilationUnit.java#newcode62 dev/core/test/com/google/gwt/dev/javac/MockCompilationUnit.java:62: return "/mock/" + typeName; On 2011/03/24 20:24:44, scottb wrote:
"/mock/" + Shared.toPath(typeName)
(Will need to update unit tests.)
Done. http://gwt-code-reviews.appspot.com/1375802/diff/10040/dev/core/test/com/google/gwt/dev/javac/PersistentUnitCacheTest.java File dev/core/test/com/google/gwt/dev/javac/PersistentUnitCacheTest.java (right): http://gwt-code-reviews.appspot.com/1375802/diff/10040/dev/core/test/com/google/gwt/dev/javac/PersistentUnitCacheTest.java#newcode26 dev/core/test/com/google/gwt/dev/javac/PersistentUnitCacheTest.java:26: * Unit test for {@link PersistentUnitCache} On 2011/03/24 20:24:44, scottb wrote:
period
Done. http://gwt-code-reviews.appspot.com/1375802/diff/10040/dev/core/test/com/google/gwt/dev/javac/PersistentUnitCacheTest.java#newcode33 dev/core/test/com/google/gwt/dev/javac/PersistentUnitCacheTest.java:33: deleteDirRecursive(lastCacheDir); On 2011/03/24 20:24:44, scottb wrote:
Reuse Util.recursiveDelete(lastCacheDir, false)
Done. http://gwt-code-reviews.appspot.com/1375802/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
