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

List:       webkit-dev
Subject:    Re: [webkit-dev] Use BlinkMergeCandidate to merge/back port Blink changes
From:       Ryosuke Niwa <rniwa () webkit ! org>
Date:       2013-05-23 17:52:19
Message-ID: CABNRm62sm=rX6zX7YQ9nJ06oAKfPPzy0xRnjigQ+jKCokzNAhw () mail ! gmail ! com
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


I always post patches as my patch saying I'm merging some patch with a
chromium.googlesource.com URL as in:
http://trac.webkit.org/changeset/150416

In a lot of cases, I find myself fixing the patch or rewriting the patch
because either the patch is wrong, doesn't conform to the WebKit style, or
it can be improved.

- R. Niwa

On Thu, May 23, 2013 at 9:49 AM, Andreas Kling <akling@apple.com> wrote:

> On May 23, 2013, at 6:26 PM, Darin Adler <darin@apple.com> wrote:
>
> On May 23, 2013, at 8:42 AM, Sergio Villar Senin <svillar@igalia.com>
> wrote:
>
> There is an interesting question about merging fixes from Blink. Should w=
e
> keep the original author in the ChangeLog appending the name of the merge=
r
> maybe?
>
>
> Generally speaking, the person committing to WebKit is responsible for th=
e
> content of the patch. I prefer not to think of that person as the =93merg=
er=94.
> Their name should still be on the patch.
>
> And yes, I think it=92s handy to cite the name of the author of the Blink
> patch in the change log.
>
>
> That=92s along the lines of the approach I=92ve been taking.
>
> For patches that meet both of these criteria:
>
> - Either trivial, or in my area of expertise.
> - Merges cleanly without much/any modification
>
> ...I put on my reviewer hat and pretend it=92s a WebKit patch, review it =
and
> land it.
> Example: <http://trac.webkit.org/changeset/149734>
>
> For patches I=92m less comfortable with, or that just need more work to f=
it
> into WebKit, I put on my committer hat, wrap it into a new patch, and add
> it to the review queue.
> Example: <http://trac.webkit.org/changeset/149110>
>
> In both cases, I credit the Blink author and provide a URL to the Blink
> change-set.
>
> -Andreas
>
> _______________________________________________
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
>
>

[Attachment #5 (text/html)]

<div dir="ltr">I always post patches as my patch saying I&#39;m merging some patch \
with a <a href="http://chromium.googlesource.com" \
target="_blank">chromium.googlesource.com</a> URL as in:<div><a \
href="http://trac.webkit.org/changeset/150416" \
target="_blank">http://trac.webkit.org/changeset/150416</a></div>

<div><br></div><div style>In a lot of cases, I find myself fixing the patch or \
rewriting the patch because either the patch is wrong, doesn&#39;t conform to the \
WebKit style, or it can be improved.</div><div><br></div><div class="gmail_extra">

<div>- R. Niwa</div>
<br><div class="gmail_quote">On Thu, May 23, 2013 at 9:49 AM, Andreas Kling <span \
dir="ltr">&lt;<a href="mailto:akling@apple.com" \
target="_blank">akling@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 style="word-wrap:break-word"><div>On May 23, 2013, at 6:26 PM, Darin Adler \
&lt;<a href="mailto:darin@apple.com" target="_blank">darin@apple.com</a>&gt; \
wrote:<br><div><br><blockquote type="cite"><div \
style="letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">



On May 23, 2013, at 8:42 AM, Sergio Villar Senin &lt;<a \
href="mailto:svillar@igalia.com" target="_blank">svillar@igalia.com</a>&gt; \
wrote:<br><br><blockquote type="cite">There is an interesting question about merging \
fixes from Blink. Should we keep the original author in the ChangeLog appending the \
name of the merger maybe?<br>


</blockquote><br>Generally speaking, the person committing to WebKit is responsible \
for the content of the patch. I prefer not to think of that person as the “merger”. \
Their name should still be on the patch.<br><br>And yes, I think it’s handy to cite \
the name of the author of the Blink patch in the change log.</div>


</blockquote></div><div dir="auto"><br></div></div><div dir="auto">That’s along the \
lines of the approach I’ve been taking.</div><div dir="auto"><br></div><div \
dir="auto">For patches that meet both of these criteria:</div>


<div dir="auto"><br></div><div dir="auto">- Either trivial, or in my area of \
expertise.</div><div dir="auto">- Merges cleanly without much/any \
modification</div><div dir="auto"><br></div><div dir="auto">...I put on my reviewer \
hat and pretend it’s a WebKit patch, review it and land it.</div>


<div dir="auto">Example: &lt;<a href="http://trac.webkit.org/changeset/149734" \
target="_blank">http://trac.webkit.org/changeset/149734</a>&gt;</div><div \
dir="auto"><br></div><div dir="auto">For patches I’m less comfortable with, or that \
just need more work to fit into WebKit, I put on my committer hat, wrap it into a new \
patch, and add it to the review queue.</div>


<div dir="auto">Example: &lt;<a href="http://trac.webkit.org/changeset/149110" \
target="_blank">http://trac.webkit.org/changeset/149110</a>&gt;</div><div \
dir="auto"><br></div><div dir="auto">In both cases, I credit the Blink author and \
provide a URL to the Blink change-set.</div>


<span><font color="#888888"><div dir="auto"><br></div><div \
dir="auto">-Andreas</div></font></span></div><br>_______________________________________________<br>
 webkit-dev mailing list<br>
<a href="mailto:webkit-dev@lists.webkit.org" \
target="_blank">webkit-dev@lists.webkit.org</a><br> <a \
href="https://lists.webkit.org/mailman/listinfo/webkit-dev" \
target="_blank">https://lists.webkit.org/mailman/listinfo/webkit-dev</a><br> \
<br></blockquote></div><br></div></div>



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


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

Configure | About | News | Add a list | Sponsored by KoreLogic