Brian Slesinsky has posted comments on this change.
Change subject: Adds part of Java 7 new language features.
......................................................................
Patch Set 8: Code-Review+1
(7 comments)
Looks fine to me. I have a few nits, but they don't need to be fixed.
....................................................
File
dev/core/src/com/google/gwt/dev/javac/testing/impl/Java7MockResources.java
Line 22: public static final MockJavaResource INTEGERLITERALS = new
MockJavaResource(
Nit: INTEGER_LITERALS to match the use of underscores elsewhere in this
file.
More nits: also, some of the constants match the class being created
(INTEGER_LITERALS matches IntegerLiterals) but others don't. Some classes
end with "Test" and others don't. It would be a bit cleaner to make them
match.
Line 26: StringBuilder code = new StringBuilder();
This sort of code could be made slightly more concise with a helper method:
/**
* Concatenate lines of text, adding a newline to the end of each string.
*/
static String concat(String... lines);
But it's fine as-is.
....................................................
File dev/core/src/com/google/gwt/dev/util/arg/OptionSource.java
Line 26: enum SourceLevel {
Maybe SourceLevel should be a top-level enum? It would make imports shorter
by default.
Line 27: _6("1.6", "6"),
perhaps JAVA6 and JAVA7 instead of underscore?
Line 39: * Returns a string value representation for the source level.
The javadoc here seems to be boilerplate. Better to give an example of what
will be returned, and perhaps say when to use "1.6" and when to use "6"?
....................................................
File dev/core/test/com/google/gwt/dev/CompilerTest.java
Line 60: assertEquals(OptionSource.SourceLevel._7,
options.getSourceLevel());
Perhaps import SourceLevel directly.
....................................................
File
user/test-super/com/google/gwt/dev/jjs/super/com/google/gwt/dev/jjs/test/Java7Test.java
Line 27: * IMPORTANT: For each test here there must exists the
corresponding method in the non super sourced
"must exist"
--
To view, visit https://gwt-review.googlesource.com/2650
To unsubscribe, visit https://gwt-review.googlesource.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I91c1f39ff20a2e7ac131d647bf4c96e34ce47a70
Gerrit-PatchSet: 8
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Roberto Lublinerman <[email protected]>
Gerrit-Reviewer: Brian Slesinsky <[email protected]>
Gerrit-Reviewer: John Stalcup <[email protected]>
Gerrit-Reviewer: Leeroy Jenkins <[email protected]>
Gerrit-Reviewer: Matthew Dempsky <[email protected]>
Gerrit-Reviewer: Matthew Dempsky <[email protected]>
Gerrit-Reviewer: Ray Cromwell <[email protected]>
Gerrit-Reviewer: Roberto Lublinerman <[email protected]>
Gerrit-Reviewer: Roberto Lublinerman <[email protected]>
Gerrit-Reviewer: Thomas Broyer <[email protected]>
Gerrit-HasComments: Yes
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
---
You received this message because you are subscribed to the Google Groups "GWT Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
