<div dir="ltr">I don't think creating a new thread (or series of N threads) because an early patch in a series necessitate some refactoring of later patches is ideal - while patch series can/should be avoided where reasonable (& if the series is small enough/the framing/design is obvious enough, you can hold on to the later patches rather than reviewing them all in parallel with the volatility that comes with that approach) - but there are certainly cases I've seen where it's helpful to send a dependent series together to help frame/justify the goals of the early refactoring patches. If the early refactoring requires changes - yeah, you get that done, signed off and committed (while possibly doing higher level design review of later patches that might be applicable regardless of the early refactoring - like if you decide to rename a newly created function in an early patch, that shouldn't hurt the review going on in later patches, for instance) & update later patches with the knock-on effects. No need to start new mail threads/lose review context/etc.<br><br>(sometimes review might indicate the whole patch series direction is not preferred - in which case, yeah, maybe throwing it out and starting a new thread/set of threads is appropriate - but not always)</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Jan 15, 2020 at 2:16 PM Renato Golin via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Wed, 15 Jan 2020 at 18:55, Nicolai Hähnle <<a href="mailto:nhaehnle@gmail.com" target="_blank">nhaehnle@gmail.com</a>> wrote:<br>
> This has simply not been true in my experience. Actually, not having<br>
> to re-send a new series is one of the main advantages that<br>
> Phabricator-based review has over the original review style for Git,<br>
> which is to send patch series via mailing lists.<br>
<br>
Interesting. If they can be committed separately, why were they a<br>
series in the first place?<br>
<br>
Or was that independent, but related, patches? In this case, they<br>
could have been different PRs with mention of each other, no?<br>
<br>
<br>
> It might be the case that you occasionally have series where major<br>
> redesigns are required, and then asking for a fresh start makes sense.<br>
<br>
What I mean by patch series is exactly what you would have in a<br>
sanitised git branch: a number of sequential patches that build on<br>
each other. You cannot commit patch 3 before 2 as Git wouldn't let you<br>
merge.<br>
<br>
In those cases, if we want to have patch 3 before the series, the<br>
author needs to rewrite history (rebase -i), push the patch up and<br>
pull into another branch, so that we can have the old tree with -1<br>
patches as the derived series, and the cherry-picked patch merged<br>
sooner.<br>
<br>
But in those cases, the patch would most certainly have to be<br>
different, and that would also change the remaining series. So, new<br>
patch, new series.<br>
<br>
Makes sense?<br>
<br>
--renato<br>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div>