Open Stack

On 8/7/2014 2:54 AM, Thierry Carrez wrote:
> Brant Knudson wrote:
>> There seems to be a difference of opinion on whether a patch series
>> should be squashed or not when it's backported to the stable branch.
>> What's everyone's opinion on this? Whatever the result is we should
>> update https://wiki.openstack.org/wiki/StableBranch#Proposing_Fixes .
>>
>> This might affect some of us more than others because I like to split my
>> bug fixes into first a patch that shows the bug and then a patch that
>> fixes it, so I've generally always got 2 patches to backport.
>>
>> As someone that works on a project that involves backporting changes to
>> branches that have been abandoned by community it's probably easier to
>> backport a squashed patch. And the note for the OSSA is going to have
>> fewer reviews and be less confusing to the non-initiated if there's only
>> 1 commit.
>
> Keeping them separate usually makes it easier to compare with the master
> patch. One trick with stable branch reviews is that we don't judge the
> quality of the original patch (this is what the master review by core
> reviewers is for). We vouch that the same patch has landed in the master
> branch, and that it is acceptable for the stable branch.
>
> IMHO the first part of the task (comparing stable patches with master
> patches) is facilitated by proposing similarly-organized series... and
> since the stable review is simpler than the master review, the number of
> patches is not that much of an issue ?
>

The only time I've squashed a series in a backport is when two or more 
patches really need to land together because one patch fixes part of a 
bug but might introduce another bug, which is fixed by another later 
change.  We had a series of 4 changes to a nova/neutron interaction that 
I backported to stable/icehouse awhile back and rather than just make 
sure they all got merged at the same time, I squashed them and kept the 
original commit messages from each patch in the single commit.

It does make it harder on the backport reviewer but it avoids merging 
something half-assed.

-- 

Thanks,

Matt Riedemann