<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Jan 15, 2020 at 3:29 PM Renato Golin <<a href="mailto:rengolin@gmail.com">rengolin@gmail.com</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 23:19, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br>
> 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>
On Git, I normally would have done that on separate, but related,<br>
branches and pull requests.</blockquote><div><br>If the patches aren't dependent on each other - then that's not, to me, the situation we're discussing. That situation can/should always be handled as you say - separate branches/reviews/etc. Maybe related (link to the other reviews for context/design understanding/etc) but when talking about reviewing a patch series, at least I assume that means a set of /dependent/ patches, where later patches cannot be committed without earlier patches.<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> II find it much easier to work with<br>
rebases and cherry-picks across branches than on the same branch. A<br>
rebase -i can turn dreadful if the same part of the code is touched by<br>
more than one patch, and it can be almost impossible if that pattern<br>
happens more than once.<br>
<br>
In my use of the term, a patch series is a mandated order of directly<br>
related commits, like you would have in a branch, if the patches are<br>
logically separated into clear individual steps, for the clarity of<br>
review.<br>
<br>
Perhaps the name clash has caused more trouble than the original<br>
discussion. If that's not what most people would have thought,<br>
apologies for the confusion.<br></blockquote><div><br>I'm not sure where the idea that a patch series is anything other than that ^ came from. When I was talking about a patch series, it was/is with that definition in mind - ordered/dependent commits. I said "dependent series" to reinforce this idea that the kind of situation I was describing was one in which later patches in the series depend on the changes in earlier patches.<br><br>- Dave<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
--renato<br>
</blockquote></div></div>