LGTM w/ nits.

http://gwt-code-reviews.appspot.com/1359802/diff/4/5
File dev/core/src/com/google/gwt/dev/javac/BytecodeSignatureMaker.java
(right):

http://gwt-code-reviews.appspot.com/1359802/diff/4/5#newcode160
dev/core/src/com/google/gwt/dev/javac/BytecodeSignatureMaker.java:160:
int mask =
Make into a class-level constant?

http://gwt-code-reviews.appspot.com/1359802/diff/4/5#newcode161
dev/core/src/com/google/gwt/dev/javac/BytecodeSignatureMaker.java:161:
~(Opcodes.ACC_ANNOTATION | Opcodes.ACC_DEPRECATED
I think Annotation might matter; an interface is not allowed to extend
an annotation interface.

http://gwt-code-reviews.appspot.com/1359802/diff/4/5#newcode202
dev/core/src/com/google/gwt/dev/javac/BytecodeSignatureMaker.java:202:
public String getClassDependencySignature(byte[] byteCode) {
Make static?

http://gwt-code-reviews.appspot.com/1359802/diff/4/6
File
dev/core/test/com/google/gwt/dev/javac/BytecodeSignatureMakerTest.java
(right):

http://gwt-code-reviews.appspot.com/1359802/diff/4/6#newcode590
dev/core/test/com/google/gwt/dev/javac/BytecodeSignatureMakerTest.java:590:
private boolean compareUnitSignature(String originalSignature,
If you formulate this as assertSignaturesEqual/NotEqual and call
assertEquals() internally, you get a clearer exception than
assertTrue/False gives.  (Although, visually seeing the difference in
hashes wouldn't be that useful; you'd want to compare the results of a
for-testing BytecodeSignatureMaker.getRawSignature().)

http://gwt-code-reviews.appspot.com/1359802/show

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Reply via email to