Re: [webkit-dev] GTK Skiplist

2010-09-03 Thread Steve Block
I think the code review in which the need to do this was first pointed
out to me was https://bugs.webkit.org/show_bug.cgi?id=27716#c32

I've been following this pattern for the Geolocation and
DeviceOrientation tests, all of which are skipped on GTK. If I can
instead list the directory, that would mean one fewer file to modify
each time a new test is added.

Steve

-- 
Google UK Limited
Registered Office: Belgrave House, 76 Buckingham Palace Road, London SW1W 9TQ
Registered in England Number: 3977902
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] GTK Skiplist

2010-09-03 Thread Xan Lopez
On Fri, Sep 3, 2010 at 9:50 PM, Jeremy Orlow jor...@chromium.org wrote:
 Recently in a code review for IndexedDB, I was told that I should be adding
 every test individually to GTK's skip list.  The reason cited was this
 comment at the top # Also, no grouping should occur. Every test for
 itself.  For features like IndexedDB which are not turned on by a port, is
 it still expected that I should be adding each and every test to the GTK
 skip list?  If so, why?  /storage/indexeddb has been in the skip list for
 some time and seems to work fine, at least from a technical standpoint.

I didn't write that, but IMHO it's perfectly OK to skip whole
directories when the reason for skipping all the tests inside is the
same or essentially the same. We already do this for storage/, for
instance.

Xan

 Thanks,
 Jeremy
 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] GTK Skiplist

2010-09-03 Thread Jeremy Orlow
I promise to only put IndexedDB tests in the indexeddb directory.  :-)

On Fri, Sep 3, 2010 at 5:52 PM, Darin Adler da...@apple.com wrote:

 On Sep 3, 2010, at 5:50 AM, Jeremy Orlow wrote:

  Recently in a code review for IndexedDB, I was told that I should be
 adding every test individually to GTK's skip list.

 That doesn’t sound quite right. One of the reasons the tests are grouped
 into directories is so that things like the Skipped list can take advantage
 of the categories created.

 However, we don’t want to disable tests just because someone made an
 unfortunate choice of what directory to put them in. Lets make sure that
 doesn’t happen!

-- Darin


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] GTK Skiplist

2010-09-03 Thread Jeremy Orlow
On Fri, Sep 3, 2010 at 6:15 PM, David Levin le...@chromium.org wrote:

 It looks like I made a comment (that I don't even remember at this point :)
 ) which may have triggered this.

 fwiw, I try to respect how other platforms are maintaining their code
 because I'd like other people to do the same even if they may think may
 platform is odd.


There is of course a balance to be struck between the freedoms of a port to
do things the way it likes with the burden it places on the community.  (I
say this knowing that Chromium has its equivalent of a Skiplist in a
completely different format.  But we do this for a very good reason, and I
believe work is under way to unify things.)


 In this case, what is desired is very clearly spelled out in the file:
 http://trac.webkit.org/browser/trunk/LayoutTests/platform/gtk/Skipped#L4


Sure, but as is already clear by Xan and Darin's response, it seems that the
situation may not be as clear as the text makes it sound.


 I don't know why that was added because I didn't do it :). If one disagrees
 with that comment, it seems like a very reasonable path to take would be to
 figure out who wrote that (which isn't hard in git or svn) and* take it up
 with them/the reviewer of that patch to understand if there is some reason
 for this peculiarity* (and then maybe clarify the comment in some way).


The only thing I may have done wrong here is spam a list when a direct email
could have done.  But even then, the answer would have probably warranted a
PSA style email to the list (given that many of us are apparently not on the
same page with this policy).  And such an email very well could have kicked
off a debate of its own.  So I felt like it was worth doing this in a
broadcast manner.  (If it were some nit in a particular part of a code, I
would have done it differently.)

Btw, I find it humorous that, by your own logic, you probably should have
sent your rebuke in a private email.  :-)

J


 dave

 On Fri, Sep 3, 2010 at 9:54 AM, Jeremy Orlow jor...@chromium.org wrote:

 I promise to only put IndexedDB tests in the indexeddb directory.  :-)

 On Fri, Sep 3, 2010 at 5:52 PM, Darin Adler da...@apple.com wrote:

 On Sep 3, 2010, at 5:50 AM, Jeremy Orlow wrote:

  Recently in a code review for IndexedDB, I was told that I should be
 adding every test individually to GTK's skip list.

 That doesn’t sound quite right. One of the reasons the tests are grouped
 into directories is so that things like the Skipped list can take advantage
 of the categories created.

 However, we don’t want to disable tests just because someone made an
 unfortunate choice of what directory to put them in. Lets make sure that
 doesn’t happen!

-- Darin



 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev



___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev