John, thanks for all the feedback. I think this is done.
Bob, I've expanded it to make the relocated RequestFactory classes use
the new event package, along with DynaTableRF. Could you pay particular
attention to RequestFactoryJarExtractor? I'm not sure if I did the right
thing there.
Even with this and the autobean patch we're maybe not quite done. Try
running:
find . -name \*.java | xargs grep -l "import com.google.gwt" | grep -v
impl
It's 90% autobean, but not entirely. I suppose we're okay if none of
these are due to public methods, but we should look carefully. (And note
that I restricted that search to avoid the impl directories. If we
decide we care about those I imagine there are more issues.)
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/src/com/google/gwt/activity/shared/Activity.java
File user/src/com/google/gwt/activity/shared/Activity.java (right):
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/src/com/google/gwt/activity/shared/Activity.java#newcode19
user/src/com/google/gwt/activity/shared/Activity.java:19: import
com.google.gwt.user.client.ui.IsWidget;
On 2011/04/01 19:19:31, jlabanca wrote:
This looks like an unused import (only used in JavaDoc). Might be
better to
inline the full path in the JavaDoc to avoid the warning.
Done.
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/src/com/google/gwt/event/Event.gwt.xml
File user/src/com/google/gwt/event/Event.gwt.xml (right):
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/src/com/google/gwt/event/Event.gwt.xml#newcode2
user/src/com/google/gwt/event/Event.gwt.xml:2: <inherits
name="com.google.gwt.event.EventBase" />
On 2011/04/01 19:19:31, jlabanca wrote:
Remove tabs from all lines or revert.
Done.
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/src/com/google/gwt/event/EventBase.gwt.xml
File user/src/com/google/gwt/event/EventBase.gwt.xml (right):
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/src/com/google/gwt/event/EventBase.gwt.xml#newcode5
user/src/com/google/gwt/event/EventBase.gwt.xml:5: <source path="shared"
/>
On 2011/04/01 19:19:31, jlabanca wrote:
remove tabs from all lines.
Done.
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/src/com/google/web/bindery/event/Event.gwt.xml
File user/src/com/google/web/bindery/event/Event.gwt.xml (right):
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/src/com/google/web/bindery/event/Event.gwt.xml#newcode17
user/src/com/google/web/bindery/event/Event.gwt.xml:17: <source
path="client"/>
On 2011/04/01 19:19:31, jlabanca wrote:
The client path is not actually present. Can we remove it? Is it
here for
future support?
Done.
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/src/com/google/web/bindery/event/Event.gwt.xml#newcode18
user/src/com/google/web/bindery/event/Event.gwt.xml:18: <source
path="shared"/>
On 2011/04/01 19:19:31, jlabanca wrote:
extra spaces or tabs
Done.
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/src/com/google/web/bindery/event/shared/Event.java
File user/src/com/google/web/bindery/event/shared/Event.java (right):
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/src/com/google/web/bindery/event/shared/Event.java#newcode2
user/src/com/google/web/bindery/event/shared/Event.java:2: * Copyright
2009 Google Inc.
On 2011/04/01 19:19:31, jlabanca wrote:
2011
Done.
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/src/com/google/web/bindery/event/shared/EventBus.java
File user/src/com/google/web/bindery/event/shared/EventBus.java (right):
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/src/com/google/web/bindery/event/shared/EventBus.java#newcode2
user/src/com/google/web/bindery/event/shared/EventBus.java:2: *
Copyright 2010 Google Inc.
On 2011/04/01 19:19:31, jlabanca wrote:
2011
Done.
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/src/com/google/web/bindery/event/shared/HandlerRegistration.java
File
user/src/com/google/web/bindery/event/shared/HandlerRegistration.java
(right):
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/src/com/google/web/bindery/event/shared/HandlerRegistration.java#newcode20
user/src/com/google/web/bindery/event/shared/HandlerRegistration.java:20:
* {@link EventBus#addHandler}, used to deregister.
On 2011/04/01 19:19:31, jlabanca wrote:
Close parathesis from (e.g....):
(e.g. via {@linkEventBus#addHandler})
Done.
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/src/com/google/web/bindery/event/shared/HandlerRegistration.java#newcode22
user/src/com/google/web/bindery/event/shared/HandlerRegistration.java:22:
* A tip: to make a handler deregister itself, the following works:
Tweaked, but I like it as a tip.
Respectfully declined.On 2011/04/01 19:19:31, jlabanca wrote:
Phrasing. How about the following:
NOTE: The following code creates a handler that deregisters itself the
first
time it handles an event.
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/src/com/google/web/bindery/event/shared/HandlerRegistration.java#newcode35
user/src/com/google/web/bindery/event/shared/HandlerRegistration.java:35:
* Deregisters a handler.
Nice, thanks.
On 2011/04/01 19:19:31, jlabanca wrote:
The API should call out how to handle multiple calls or calls to
handlers that
are already removed.
Deregisters the handler associated with this registration object if
the handler
is still attached to the event source. If the handler is no longer
attached to
the event source, this is a no-op.
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/src/com/google/web/bindery/event/shared/ResettableEventBus.java
File
user/src/com/google/web/bindery/event/shared/ResettableEventBus.java
(right):
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/src/com/google/web/bindery/event/shared/ResettableEventBus.java#newcode2
user/src/com/google/web/bindery/event/shared/ResettableEventBus.java:2:
* Copyright 2010 Google Inc.
On 2011/04/01 19:19:31, jlabanca wrote:
2011
Done.
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/src/com/google/web/bindery/event/shared/ResettableEventBus.java#newcode27
user/src/com/google/web/bindery/event/shared/ResettableEventBus.java:27:
public class ResettableEventBus extends EventBus {
On 2011/04/01 19:19:31, jlabanca wrote:
Make sure you incorporate http://gwt-code-reviews.appspot.com/1388804/
before
submitting.
Done.
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/src/com/google/web/bindery/event/shared/SimpleEventBus.java
File user/src/com/google/web/bindery/event/shared/SimpleEventBus.java
(right):
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/src/com/google/web/bindery/event/shared/SimpleEventBus.java#newcode2
user/src/com/google/web/bindery/event/shared/SimpleEventBus.java:2: *
Copyright 2010 Google Inc.
On 2011/04/01 19:19:31, jlabanca wrote:
2011
Done.
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/src/com/google/web/bindery/event/shared/SimpleEventBus.java#newcode37
user/src/com/google/web/bindery/event/shared/SimpleEventBus.java:37:
private interface Command {
On 2011/04/01 19:19:31, jlabanca wrote:
interfaces go above fields.
Done.
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/src/com/google/web/bindery/event/shared/SimpleEventBus.java#newcode69
user/src/com/google/web/bindery/event/shared/SimpleEventBus.java:69:
protected SimpleEventBus(boolean fireInReverseOrder) {
Nice idea. Thanks.
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/src/com/google/web/bindery/event/shared/SimpleEventBus.java#newcode96
user/src/com/google/web/bindery/event/shared/SimpleEventBus.java:96: }
On 2011/04/01 19:19:31, jlabanca wrote:
You can move the type and handler checks to doAdd(). Slight reduction
in code
size and protects against future uses of doAdd().
The source==null check will have to stay in this method.
Done.
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/src/com/google/web/bindery/event/shared/SimpleEventBus.java#newcode116
user/src/com/google/web/bindery/event/shared/SimpleEventBus.java:116: }
On 2011/04/01 19:19:31, jlabanca wrote:
Ditto. Move event==null check into doFire, keep the source==null
check in this
method.
Also, there are a lot of if(null) checks. You could add a
assertNotNull(Object
toCheck, String message)
Done.
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/src/com/google/web/bindery/event/shared/SimpleEventBus.java#newcode124
user/src/com/google/web/bindery/event/shared/SimpleEventBus.java:124:
protected <H> void doRemove(Event.Type<H> type, Object source, H
handler) {
On 2011/04/01 19:19:31, jlabanca wrote:
I don't like adding deprecated APIs. Can you use the violator pattern
to keep
the deprecated methods out of here?
Done.
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/src/com/google/web/bindery/event/shared/UmbrellaException.java
File user/src/com/google/web/bindery/event/shared/UmbrellaException.java
(right):
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/src/com/google/web/bindery/event/shared/UmbrellaException.java#newcode2
user/src/com/google/web/bindery/event/shared/UmbrellaException.java:2: *
Copyright 2010 Google Inc.
On 2011/04/01 19:19:31, jlabanca wrote:
2011
Done.
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/src/com/google/web/bindery/event/shared/UmbrellaException.java#newcode22
user/src/com/google/web/bindery/event/shared/UmbrellaException.java:22:
* {@link Throwable}s together. Typically thrown after loop, with all of
the
On 2011/04/01 19:19:31, jlabanca wrote:
thrown after loop => thrown after a loop
Done.
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/src/com/google/web/bindery/event/shared/testing/CountingEventBus.java
File
user/src/com/google/web/bindery/event/shared/testing/CountingEventBus.java
(right):
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/src/com/google/web/bindery/event/shared/testing/CountingEventBus.java#newcode77
user/src/com/google/web/bindery/event/shared/testing/CountingEventBus.java:77:
counts.put(type, count - 1);
On 2011/04/01 19:19:31, jlabanca wrote:
maybe just getCount(type) - 1
Done, slightly more involved due to int v. Integer.
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/src/com/google/web/bindery/event/shared/testing/CountingEventBus.java#newcode85
user/src/com/google/web/bindery/event/shared/testing/CountingEventBus.java:85:
counts.put(type, count + 1);
On 2011/04/01 19:19:31, jlabanca wrote:
getCount(type) + 1
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/test/com/google/gwt/event/shared/SimpleEventBusTest.java
File user/test/com/google/gwt/event/shared/SimpleEventBusTest.java
(left):
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/test/com/google/gwt/event/shared/SimpleEventBusTest.java#oldcode35
user/test/com/google/gwt/event/shared/SimpleEventBusTest.java:35: public
void testAddAndRemoveHandlers() {
They test those deprecated package protected methods you dislike so
much, and which are no longer provided by this gutted shell of a class.
They are still present in HandlerManagerTest, so no coverage is lost.
On 2011/04/01 19:19:31, jlabanca wrote:
Why remove all of these tests?
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/test/com/google/web/bindery/event/shared/EventBusTestBase.java
File user/test/com/google/web/bindery/event/shared/EventBusTestBase.java
(right):
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/test/com/google/web/bindery/event/shared/EventBusTestBase.java#newcode2
user/test/com/google/web/bindery/event/shared/EventBusTestBase.java:2: *
Copyright 2008 Google Inc.
On 2011/04/01 19:19:31, jlabanca wrote:
2011
Done.
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/test/com/google/web/bindery/event/shared/ResettableEventBusTest.java
File
user/test/com/google/web/bindery/event/shared/ResettableEventBusTest.java
(right):
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/test/com/google/web/bindery/event/shared/ResettableEventBusTest.java#newcode2
user/test/com/google/web/bindery/event/shared/ResettableEventBusTest.java:2:
* Copyright 2010 Google Inc.
On 2011/04/01 19:19:31, jlabanca wrote:
2011
Done.
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/test/com/google/web/bindery/event/shared/SimpleEventBusTest.java
File
user/test/com/google/web/bindery/event/shared/SimpleEventBusTest.java
(right):
http://gwt-code-reviews.appspot.com/1394803/diff/1/user/test/com/google/web/bindery/event/shared/SimpleEventBusTest.java#newcode2
user/test/com/google/web/bindery/event/shared/SimpleEventBusTest.java:2:
* Copyright 2010 Google Inc.
On 2011/04/01 19:19:31, jlabanca wrote:
2011
Done.
http://gwt-code-reviews.appspot.com/1394803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
