[webkit-dev] Review Queue

[Yong Li]

Hi Adam,

I'm a very first user of check-webkit-style (was called cpplint when I used 
it), and reported some bugs of it. The current indentation issue of some 
construtor inialization list were actually introduced by following the 
suggestion of old version cpplint. It must have been improved a lot, so I 
should run it again.

Thank you so much for those comments. I like most of them. But I have 
questions about something:

1. one class one file

I agree that it's a good style to create a file for each class. But 
sometimes the class is very light, and is only useful to one module, in this 
case putting it in the same file won't hurt anything. Second, there're a lot 
of such cases in webkit existing files, including some committed in near 
future. So why is it the reason for r- bug 28167?

2. static initializer forbidden

I'm not aware of this. Is it added in webkit code guidline?

Most the patches are for WINCE port only, and they won't affect any other 
existing port. Also, they are not read-only after being committed. So if we 
think some of them should be refactored or extracted out for sharing, we can 
always do it in the future. The png encoder of skia can definitely be shared 
by other ports, but the files were still committed to image-encoders\skia. 
Why are people requiring so much for brand-new wince port?

Thanks a lot.

-Yong

----- Original Message ----- 
From: "Adam Barth" <abarth at webkit.org>
To: "Yong Li" <yong.li at torchmobile.com>
Cc: "Eric Seidel" <eric at webkit.org>; "WebKit Development" 
<webkit-dev at lists.webkit.org>
Sent: Wednesday, September 02, 2009 9:36 AM
Subject: Re: [webkit-dev] Review Queue


Hi Yong,

Sorry for r-ing a bunch of your patches last night.  I tried to give
you constructive comments.  As you might be aware, we have a style
linter now:

./WebKitTools/Scripts/check-webkit-style

It might help us skip past some of the trivial review comments (like
improper indenting of initializer lists).  For the patches with
licensing issues, I'm not sure how best to proceed.

Adam


On Wed, Sep 2, 2009 at 8:29 AM, Yong Li<yong.li at torchmobile.com> wrote:
> Thanks a lot for spending time on reviewing. As a developer, I hope I can
> have more constructive comments for r-. I understand it's a hard work and
> truly appreciate all of you who spend time on it.
>
> -Yong
>
> ----- Original Message -----
> From: Eric Seidel
> To: WebKit Development
> Sent: Wednesday, September 02, 2009 6:10 AM
> Subject: [webkit-dev] Review Queue
> Just your friendly reminder that the review-queue is out of control again.
> We're @ 54 (was 84 on Monday):
> https://bugs.webkit.org/buglist.cgi?field0-0-0=flagtypes.name&type0-0-0=equals&value0-0-0=review%3F
> If a reviewers could each take a stab at r-'ing (or r+'ing!) a few patches
> today we'd be back under 30 in no time...
> -eric
> p.s. Thanks to Adam Barth (and others) for applying the flaming sword of 
> r-
> last night. It's slightly more under control.
>
> ________________________________
>
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>
>