Squash patch series when backport?
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. - Brant
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 ? -- Thierry Carrez (ttx)
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
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512 On 07/08/14 14:19, Matt Riedemann wrote:
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.
This is a valid case to squash. Other valid option is when gate was broken by some external changes that require gate to be run against multiple patches at once to pass it. Other than that, I expect backport patches to be identical to what is in master [obviously, adding conflict resolution and adoption whenever needed]. If anything, this facilitates patch comparison. It also makes granular reverts easier, in case such need arises in the future. /Ihar -----BEGIN PGP SIGNATURE----- Version: GnuPG/MacGPG2 v2.0.22 (Darwin) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCgAGBQJT46FYAAoJEC5aWaUY1u57AJYIAJeIHg+3JKSuOgjz7bNDRl22 v6c+o3NTkVwZZIpw4A0DJH1E7AW0UoHNXmj7Ii+ILHASjAnt5SLsggAja2S4/QOb XT+SQGouhvUNNsrASxWSAV4gP+IwLk5/hbV7tzc9Hpg1v+pI68779NSw8KCA+gPZ MPm7mVw/Lfz51v4HO8kbInoIOOgqW3L/bIHnoKLPVTYm+Figs0b08ESmQwjVla0b WUTnIV+4U6XARyXvCL7eGlxvhzaNumIvOT5VMuW1AirghD6mSdu0pbORnBFAomFS 2Jqnl0xMT4IsnrALkIO/5LTFRtc6xJakorOFG4YTr7QFlmyNyesQjD8AsAZ2dxE= =9WIJ -----END PGP SIGNATURE-----
I’m also a fan of squashed patches when the first introduces a second bug. Vish On Aug 7, 2014, at 8:55 AM, Ihar Hrachyshka <ihrachys@redhat.com> wrote:
Signed PGP part On 07/08/14 14:19, Matt Riedemann wrote:
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.
This is a valid case to squash. Other valid option is when gate was broken by some external changes that require gate to be run against multiple patches at once to pass it.
Other than that, I expect backport patches to be identical to what is in master [obviously, adding conflict resolution and adoption whenever needed]. If anything, this facilitates patch comparison. It also makes granular reverts easier, in case such need arises in the future.
/Ihar
_______________________________________________ Openstack-stable-maint mailing list Openstack-stable-maint@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-stable-maint
participants (5)
-
Brant Knudson
-
Ihar Hrachyshka
-
Matt Riedemann
-
Thierry Carrez
-
Vishvananda Ishaya