Thank, I moved pretty much all of the impl stuff, and some for generated use only Interfaces.
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}. On 2010/08/23 13:19:15, bobv wrote:
If this is only used by generated classes, move it into a client.impl
package. Done. 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; On 2010/08/23 13:19:15, bobv wrote:
Sort order.
Done. 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(); The auto-formater is still slightly off, but this is better. 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. On 2010/08/23 13:19:15, bobv wrote:
objects
Done. 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 On 2010/08/23 13:19:15, bobv wrote:
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.
Done. 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> On 2010/08/23 13:19:15, bobv wrote:
"the type described be the bean descriptor" ?
Done. 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> { On 2010/08/23 13:19:15, bobv wrote:
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. This is only used in Generated code. The main entery point is the GwtValidation annotation which has a sample in the javadoc. 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: On 2010/08/23 13:19:15, bobv wrote:
Extra blank line.
Done. http://gwt-code-reviews.appspot.com/766802/diff/1/9#newcode49 user/src/com/google/gwt/validation/client/GwtValidation.java:49: @Target({TYPE}) On 2010/08/23 13:19:15, bobv wrote:
The curly-braces are unnecessary since there's only one value.
Done. http://gwt-code-reviews.appspot.com/766802/diff/1/9#newcode50 user/src/com/google/gwt/validation/client/GwtValidation.java:50: @Retention(CLASS) On 2010/08/23 13:19:15, bobv wrote:
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.
Done. http://gwt-code-reviews.appspot.com/766802/diff/1/9#newcode50 user/src/com/google/gwt/validation/client/GwtValidation.java:50: @Retention(CLASS) On 2010/08/23 13:19:15, bobv wrote:
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.
Done. 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() { On 2010/08/23 13:19:15, bobv wrote:
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. Done. 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); On 2010/08/23 13:19:15, bobv wrote:
this.descriptors = new HashSet(Arrays.asList(descriptors));
Done. http://gwt-code-reviews.appspot.com/766802/diff/1/12#newcode57 user/src/com/google/gwt/validation/metadata/PropertyDescriptorImpl.java:57: return descriptors; On 2010/08/23 13:19:15, bobv wrote:
Is this Set ever going to be exposed to the end-developer's code? If
so, it
should return an unmodifiable view.
Done. 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" /> On 2010/08/23 13:19:15, bobv wrote:
The formatting looks off. New module files should be space-indented.
Done. 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}"; On 2010/08/23 13:19:15, bobv wrote:
com.acme?
Done. http://gwt-code-reviews.appspot.com/766802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
