'Re: [webkit-dev] Reformatting-only patches being applied to trunk'

[prev in list] [next in list] [prev in thread] [next in thread] 

List:       webkit-dev
Subject:    Re: [webkit-dev] Reformatting-only patches being applied to trunk
From:       Ojan Vafai <ojan () chromium ! org>
Date:       2009-07-25 23:11:24
Message-ID: 78dc8440907251611k3fec2150s6c2a01efd596f996 () mail ! gmail ! com
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]

On Sat, Jul 25, 2009 at 4:39 PM, David Hyatt <hyatt@apple.com> wrote:

> On Jul 25, 2009, at 3:08 AM, Oliver Hunt wrote:
>
>  I've just noticed that there have been a few purely style related patches
>> being landed in the tree recently, I don't believe these are a good idea and
>> that any further reformatting only patches be rejected.
>>
>
> I completely disagree.  I see nothing wrong with patches that are purely
> style cleanup.  Someone doesn't have to be doing significant work in an area
> of code to fix bad style in that code.  We've had a problem with bad style
> lingering in some files precisely because the area of code isn't being
> touched much.  It would be great to see those files cleaned up.
>

One other advantage of getting most or all of the code base to have the
correct style is that we could hook up the new linting tool to an svn/git
precommit hook so that new style errors don't get introduced. We could also
hook it into any create-patch scripts so that patches (new) contributors
upload don't need to be manually vetted by a reviewer for style. Eventually,
when we start work on a code review tool, we could automatically flag any
style violations without reviewer oversight.

Ojan

[Attachment #5 (text/html)]

<div class="gmail_quote">On Sat, Jul 25, 2009 at 4:39 PM, David Hyatt <span \
dir="ltr">&lt;<a href="mailto:hyatt@apple.com">hyatt@apple.com</a>&gt;</span> \
wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px \
#ccc solid;padding-left:1ex;">

<div class="im">On Jul 25, 2009, at 3:08 AM, Oliver Hunt wrote:<br>
<br>
</div><div class="im"><blockquote class="gmail_quote" style="margin:0 0 0 \
.8ex;border-left:1px #ccc solid;padding-left:1ex"> I&#39;ve just noticed that there \
have been a few purely style related patches being landed in the tree recently, I \
don&#39;t believe these are a good idea and that any further reformatting only \
patches be rejected.<br> </blockquote>
<br></div>
I completely disagree.  I see nothing wrong with patches that are purely style \
cleanup.  Someone doesn&#39;t have to be doing significant work in an area of code to \
fix bad style in that code.  We&#39;ve had a problem with bad style lingering in some \
files precisely because the area of code isn&#39;t being touched much.  It would be \
great to see those files cleaned up.<br>

</blockquote><div><br></div><div>One other advantage of getting most or all of the \
code base to have the correct style is that we could hook up the new linting tool to \
an svn/git precommit hook so that new style errors don&#39;t get introduced. We could \
also hook it into any create-patch scripts so that patches (new) contributors upload \
don&#39;t need to be manually vetted by a reviewer for style. Eventually, when we \
start work on a code review tool, we could automatically flag any style violations \
without reviewer oversight.</div>

<div><br></div><div>Ojan</div></div>

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

[prev in list] [next in list] [prev in thread] [next in thread] 