[gwt-contrib] Re: Code Review: Gadget RPC demo (it works!)
LGTM just a couple of nits below. On Fri, Sep 5, 2008 at 1:52 PM, Eric Ayers [EMAIL PROTECTED] wrote: A samples/gadgetrpc/src/com/google/gwt/gadgets/sample/gadgetrpc/server/GadgetRPCServlet.java Nit: Add @SuppressWarning(serial). Did you intend to have the servletStartTime field be static? Would instance final be better? A samples/gadgetrpc/src/com/google/gwt/gadgets/sample/gadgetrpc/client/ServerInfo.java Nits: accessor methods? Add @SuppressWarning(serial) A samples/gadgetrpc/src/com/google/gwt/gadgets/sample/gadgetrpc/public/GadgetRPC.html Nit: do you need this file? -- Eric Z. Ayers - GWT Team - Atlanta, GA USA http://code.google.com/webtoolkit/ -- Miguel --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: Code Review: Gadget RPC demo (it works!)
Committed as r853 and r854. Thanks for the review. On Wed, Sep 24, 2008 at 9:59 AM, Miguel Méndez [EMAIL PROTECTED] wrote: LGTM just a couple of nits below. On Fri, Sep 5, 2008 at 1:52 PM, Eric Ayers [EMAIL PROTECTED] wrote: A samples/gadgetrpc/src/com/google/gwt/gadgets/sample/gadgetrpc/server/GadgetRPCServlet.java Nit: Add @SuppressWarning(serial). Did you intend to have the servletStartTime field be static? Would instance final be better? I wanted a value that wouldn't change between RPCs, but would change if the server was restarted (code might be updated). I think that both would accomplish that, but the instance final might change without a reload, which could be confusing. A samples/gadgetrpc/src/com/google/gwt/gadgets/sample/gadgetrpc/client/ServerInfo.java Nits: accessor methods? Add @SuppressWarning(serial) added. A samples/gadgetrpc/src/com/google/gwt/gadgets/sample/gadgetrpc/public/GadgetRPC.html Nit: do you need this file? It is used by the GadgetsRPC-shell startup file so that it doesn't just come up with an error or a blank screen. Except for that the reference might have been missing from GadgetsRPC-shell, which I just added. -- Eric Z. Ayers - GWT Team - Atlanta, GA USA http://code.google.com/webtoolkit/ --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: Code Review: Gadget RPC demo (it works!)
It all sounds good to me. On Wed, Sep 24, 2008 at 11:34 AM, Eric Ayers [EMAIL PROTECTED] wrote: Committed as r853 and r854. Thanks for the review. On Wed, Sep 24, 2008 at 9:59 AM, Miguel Méndez [EMAIL PROTECTED] wrote: LGTM just a couple of nits below. On Fri, Sep 5, 2008 at 1:52 PM, Eric Ayers [EMAIL PROTECTED] wrote: A samples/gadgetrpc/src/com/google/gwt/gadgets/sample/gadgetrpc/server/GadgetRPCServlet.java Nit: Add @SuppressWarning(serial). Did you intend to have the servletStartTime field be static? Would instance final be better? I wanted a value that wouldn't change between RPCs, but would change if the server was restarted (code might be updated). I think that both would accomplish that, but the instance final might change without a reload, which could be confusing. A samples/gadgetrpc/src/com/google/gwt/gadgets/sample/gadgetrpc/client/ServerInfo.java Nits: accessor methods? Add @SuppressWarning(serial) added. A samples/gadgetrpc/src/com/google/gwt/gadgets/sample/gadgetrpc/public/GadgetRPC.html Nit: do you need this file? It is used by the GadgetsRPC-shell startup file so that it doesn't just come up with an error or a blank screen. Except for that the reference might have been missing from GadgetsRPC-shell, which I just added. -- Eric Z. Ayers - GWT Team - Atlanta, GA USA http://code.google.com/webtoolkit/ -- Miguel --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
