http://gwt-code-reviews.appspot.com/766802/diff/1/2 File user/src/com/google/gwt/validation/client/AbstractBeanDescriptor.java (right):
http://gwt-code-reviews.appspot.com/766802/diff/1/2#newcode27 user/src/com/google/gwt/validation/client/AbstractBeanDescriptor.java:27: * Abstract BeanDescriptor for use by generated {...@link GwtBeanDescriptor}. If this is only used by generated classes, move it into a client.impl package. http://gwt-code-reviews.appspot.com/766802/diff/1/2#newcode38 user/src/com/google/gwt/validation/client/AbstractBeanDescriptor.java:38: private final Class<T> clazz; Sort order. http://gwt-code-reviews.appspot.com/766802/diff/1/3 File user/src/com/google/gwt/validation/client/AbstractGwtSpecificValidator.java (right): http://gwt-code-reviews.appspot.com/766802/diff/1/3#newcode69 user/src/com/google/gwt/validation/client/AbstractGwtSpecificValidator.java:69: context.getRootBean()).setRootBeanClass(context.getRootBeanClass()).build(); This kind of builder initialization is hard to read when the setters don't appear in a consistent horizontal position. If you say x.set() // .set() // .set() // .build(); the formatter will preserve the verb-per-line format. http://gwt-code-reviews.appspot.com/766802/diff/1/5 File user/src/com/google/gwt/validation/client/ConstraintValidatorContextImpl.java (right): http://gwt-code-reviews.appspot.com/766802/diff/1/5#newcode32 user/src/com/google/gwt/validation/client/ConstraintValidatorContextImpl.java:32: * These object are very short lived. objects http://gwt-code-reviews.appspot.com/766802/diff/1/5#newcode37 user/src/com/google/gwt/validation/client/ConstraintValidatorContextImpl.java:37: public class ConstraintValidatorContextImpl<A extends Annotation, T> implements Based on the name, this sounds like it should go in the client.impl package. Anything that goes in an .impl package stays out of the public javadoc to avoid calling attention to the code behind the curtain. http://gwt-code-reviews.appspot.com/766802/diff/1/7 File user/src/com/google/gwt/validation/client/GwtBeanDescriptor.java (right): http://gwt-code-reviews.appspot.com/766802/diff/1/7#newcode24 user/src/com/google/gwt/validation/client/GwtBeanDescriptor.java:24: * @param <T> "the type described be the bean descriptor" ? http://gwt-code-reviews.appspot.com/766802/diff/1/8 File user/src/com/google/gwt/validation/client/GwtSpecificValidator.java (right): http://gwt-code-reviews.appspot.com/766802/diff/1/8#newcode29 user/src/com/google/gwt/validation/client/GwtSpecificValidator.java:29: public interface GwtSpecificValidator<G> { If this is the main entry-point for the validation code, it would be good to add a short example to the javadoc on how to obtain an intsance of the validator. http://gwt-code-reviews.appspot.com/766802/diff/1/9 File user/src/com/google/gwt/validation/client/GwtValidation.java (right): http://gwt-code-reviews.appspot.com/766802/diff/1/9#newcode47 user/src/com/google/gwt/validation/client/GwtValidation.java:47: Extra blank line. http://gwt-code-reviews.appspot.com/766802/diff/1/9#newcode49 user/src/com/google/gwt/validation/client/GwtValidation.java:49: @Target({TYPE}) The curly-braces are unnecessary since there's only one value. http://gwt-code-reviews.appspot.com/766802/diff/1/9#newcode50 user/src/com/google/gwt/validation/client/GwtValidation.java:50: @Retention(CLASS) Every time I've put anything other than runtime retention on an annotation class, somebody files a bug saying they're trying to do some reflective stuff on the server. This annotation has no impact on the visibility of an annotation in TypeOracle nor will affect the generated JS. http://gwt-code-reviews.appspot.com/766802/diff/1/11 File user/src/com/google/gwt/validation/client/MessageAndPath.java (right): http://gwt-code-reviews.appspot.com/766802/diff/1/11#newcode45 user/src/com/google/gwt/validation/client/MessageAndPath.java:45: public String toString() { Add javadoc like /** * For debugging use only. */ unless the toString() is intended to be paired with a parseX() message. It gives us leeway in the API to not have to worry about changing the format. http://gwt-code-reviews.appspot.com/766802/diff/1/12 File user/src/com/google/gwt/validation/metadata/PropertyDescriptorImpl.java (right): http://gwt-code-reviews.appspot.com/766802/diff/1/12#newcode48 user/src/com/google/gwt/validation/metadata/PropertyDescriptorImpl.java:48: this.descriptors.add(constraintDescriptor); this.descriptors = new HashSet(Arrays.asList(descriptors)); http://gwt-code-reviews.appspot.com/766802/diff/1/12#newcode57 user/src/com/google/gwt/validation/metadata/PropertyDescriptorImpl.java:57: return descriptors; Is this Set ever going to be exposed to the end-developer's code? If so, it should return an unmodifiable view. http://gwt-code-reviews.appspot.com/766802/diff/1/13 File user/test/com/google/gwt/validation/example/ValidationExample.gwt.xml (right): http://gwt-code-reviews.appspot.com/766802/diff/1/13#newcode19 user/test/com/google/gwt/validation/example/ValidationExample.gwt.xml:19: <inherits name="com.google.gwt.user.User" /> The formatting looks off. New module files should be space-indented. http://gwt-code-reviews.appspot.com/766802/diff/1/18 File user/test/com/google/gwt/validation/example/client/NotEmpty.java (right): http://gwt-code-reviews.appspot.com/766802/diff/1/18#newcode48 user/test/com/google/gwt/validation/example/client/NotEmpty.java:48: String message() default "{com.acme.constraint.NotEmpty.message}"; com.acme? http://gwt-code-reviews.appspot.com/766802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
