On Thu, Jan 15, 2009 at 3:23 PM, Emily Crutcher <e...@google.com> wrote:
> As most of the code review comments are moot, due to the change in how the > interface is implemented, capturing the two issues left here: > > *gwt.*.foo.Foo* --> has, in general, meant getting the content of the > entire package. > > My suggestion is to add another naming convention to get the "essential" > code: > *gwt.*.foo.FooBase* --> gets just the stuff absolutely needed for the > sub-packages to work. > > So, for events > *gwt.event.Even*t --> gets all the currently defined gwt event structure. > *gwt.event.EventBase* -> gets the core infrastructure needed. > I'm not sure that I agree. In other places (most notably the User module, which is a pretty bad antipattern), pulling in a module has meant pulling in all the sub-*packages*, but not sub-*modules* (because there never *have* been sub-modules to my knowledge). The reason that this happens in User is because all of the sub-packages are *within* its source folder, like so: gwt.user User.gwt.xml RemoteService.gwt.xml ... gwt.user.client Command.java ... gwt.user.client.ui Widget.java ... gwt.user.client.rpc AsyncCallback.java ... This is an antipattern, because there's essentially no way to include, say, User.gwt.xml, without getting the *source* for RemoteService.gwt.xml (because it's underneath the gwt.user.client package, all of whose sub-packages are included implicitly by User.gwt.xml). In the Event case, we're in a totally different situation. It now looks like this: gwt.event Event.gwt.xml gwt.event.shared (I still don't understand why these are called "shared", but that's another question) EventHandler.java ... gwt.event.dom Dom.gwt.xml gwt.event.dom.client BlurEvent.java ... gwt.event.logical Logical.gwt.xml gwt.event.logical.shared BeforeSelectionEvent.java ... Notice that in this case, there's no way to accidentally get another module's source by pulling in a different module. The packages for these could just as easily be: gwt.event gwt.domevent gwt.logicalevent but that would just be ugly. The only other thing is rather then having DomEvent extend User, why don't > we have DomEvent extend com.google.gwt.user.DOM? > Because the Event object is not in DOM, it's in User. ============================ > > The reason for DefaultHandlerRegistration was because, if you look at the > javadoc, you'll see we've left ourselves the option of actually adding to > that interface. The purpose of default handler registration was to give > users a way to avoid being broken if/when that happens if they needed to > implement their own handler registration. We could replace it with a public > DelegatingHandlerRegistration though,or we could say we will definitely not > break the HandlerRegistration interface. > My mistake. I read it as being package-protected before, so it seemed like overkill. I've reverted the inlining of it into HandlerManager in the latest patch. --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
