The automatic resolution of @Source files is too magic and doesn't
address external resources.  The whole HasFindableResourceDependencies
looks unnecessary and doesn't jive with how the rest of the inputs to a
ResourceGenerator are tracked.


http://gwt-code-reviews.appspot.com/1236801/diff/39001/40010
File user/src/com/google/gwt/resources/ext/ClientBundleRequirements.java
(right):

http://gwt-code-reviews.appspot.com/1236801/diff/39001/40010#newcode50
user/src/com/google/gwt/resources/ext/ClientBundleRequirements.java:50:
* whether a configuration property by the same name exists.
This sounds sloppy.

http://gwt-code-reviews.appspot.com/1236801/diff/39001/40011
File user/src/com/google/gwt/resources/ext/ResourceContext.java (right):

http://gwt-code-reviews.appspot.com/1236801/diff/39001/40011#newcode143
user/src/com/google/gwt/resources/ext/ResourceContext.java:143:
ClientBundleRequirements getRequirements();
In order to compute the simple name of the implementing ClientBundle
type, it is necessary to know all of the permutation properties that
affect the generated code.  If the requirements object can be accessed
at arbitrary point in time, it would be possible to add a property
requirement after the GeneratorContext.tryCreate() call has been made.

http://gwt-code-reviews.appspot.com/1236801/diff/39001/40011#newcode153
user/src/com/google/gwt/resources/ext/ResourceContext.java:153: *
interface can expect to call this method successfully.
The implementation does not enforce this.

http://gwt-code-reviews.appspot.com/1236801/diff/39001/40013
File
user/src/com/google/gwt/resources/ext/SupportsGeneratorResultCaching.java
(right):

http://gwt-code-reviews.appspot.com/1236801/diff/39001/40013#newcode41
user/src/com/google/gwt/resources/ext/SupportsGeneratorResultCaching.java:41:
public interface HasFindableResourceDependencies extends
Why is this interface necessary when accumulation of source files could
just as easily be added to the Requirements interface?

http://gwt-code-reviews.appspot.com/1236801/diff/39001/40014
File
user/src/com/google/gwt/resources/rebind/context/AbstractClientBundleGenerator.java
(right):

http://gwt-code-reviews.appspot.com/1236801/diff/39001/40014#newcode233
user/src/com/google/gwt/resources/rebind/context/AbstractClientBundleGenerator.java:233:
addTypeHierarchy(superType);
This will cause redundant calls to addTypeHierarchy().

types.addAll(type.getFlattenedSupertypeHierarchy()) should be
sufficient.

http://gwt-code-reviews.appspot.com/1236801/diff/39001/40014#newcode260
user/src/com/google/gwt/resources/rebind/context/AbstractClientBundleGenerator.java:260:

Extra whitespace, here and elsewhere.

http://gwt-code-reviews.appspot.com/1236801/diff/39001/40014#newcode397
user/src/com/google/gwt/resources/rebind/context/AbstractClientBundleGenerator.java:397:
CachedPropertyInformation cpi = new CachedPropertyInformation(logger,
Use a builder pattern for this type, since the number of things that
affect dependencies is likely to increase in the future.

http://gwt-code-reviews.appspot.com/1236801/diff/39001/40014#newcode484
user/src/com/google/gwt/resources/rebind/context/AbstractClientBundleGenerator.java:484:
JRealClassType sourceRealType = (JRealClassType) sourceType;
It would be more consistent with the rest of the API to cast to a
capability interface such as HasTypeSignature here, rather than using
JRealClassType.

http://gwt-code-reviews.appspot.com/1236801/diff/39001/40014#newcode820
user/src/com/google/gwt/resources/rebind/context/AbstractClientBundleGenerator.java:820:
logger, resourceContext, method);
This is insufficient if the ResourceGenerator uses external resources
not defined via an @Source annotation.

http://gwt-code-reviews.appspot.com/1236801/diff/39001/40020
File
user/src/com/google/gwt/resources/rg/ExternalTextResourceGenerator.java
(right):

http://gwt-code-reviews.appspot.com/1236801/diff/39001/40020#newcode43
user/src/com/google/gwt/resources/rg/ExternalTextResourceGenerator.java:43:
public final class ExternalTextResourceGenerator extends
FYI: Unnur is also working on a change to ETRG that may affect its use
of properties.

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

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

Reply via email to