On Thu, Sep 18, 2008 at 3:44 PM, Emily Crutcher <[EMAIL PROTECTED]> wrote:

> Yep, that was already committed to gwt-incubator trunk, the patch is older
> then the commit.
>

Ok, so I should ignore that, right?

>
>>    - Did you read the 
>> GWTC<http://groups.google.com/group/Google-Web-Toolkit-Contributors/browse_thread/thread/866faa00c999069/703f4dfa4d5ab1b7>
>>    
>> threads<http://groups.google.com/group/Google-Web-Toolkit-Contributors/msg/eb3e5cee315a4e25>about
>>  trying to do this in GWT 1.4?  The consensus was that it was not safe
>>    to rely on being able to remove all properties, and that even if it were
>>    possible it would be fragile to future browser changes (the second thread
>>    has a test HTML file, though it is missing __proto__).  I think you will
>>    need to rely on a prefix, just as we do in FastStringMap.  At there very
>>    least, you should include "__proto__", "prototype", "constructor",
>>    "toString", "watch", "unwatch", and "valueOf" in your test.  An 
>> alternative
>>    would be to not attempt to clean up things like that and simply document
>>    that no JS-defined property can be used in the string, though that seems
>>    problematic.
>>
>> What code are you looking at? I'm not removing any properties and already
> testing watch.
>

 The JsMap.patch you sent (I duplicated watch, but the others don't appear
to be tested).  In particular, __proto__ was determined to be the
showstopper before.

>
>>    - Are you sure the complexity regarding a separate hosted-mode
>>    implementation is justified?  Aside from more opportunities for errors, it
>>    means two separate implementations are not tested to be exactly the same, 
>> so
>>    any holes in the tests are an opportunity for divergent behavior in hosted
>>    mode.
>>
>> Did you have a chance to run the benchmark I sent you?
>

Not yet, but the performance in OOPHM is not terribly relevant since this
will be in long before OOPHM is.

>
>>    - Missing newlines at the end of all the .gwt.xml files.
>>
>> Our gwt.xml are supposed to have an extra new line at the end of them?
>

Every line should be terminated, but here the last one isn't.

-- 
John A. Tamplin
Software Engineer (GWT), Google

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

Reply via email to