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
-~----------~----~----~----~------~----~------~--~---

Reply via email to