[Gluster-Maintainers] Lock down period merge process

[Shyam Ranganathan]

On 08/09/2018 12:29 AM, Nigel Babu wrote:
> I would trust tooling that prevents merges rather than good faith. I
> have worked on projects where we trust good faith, but still enforce
> that with tooling[1]. It's highly likely for one or two committers to be
> unaware of an ongoing lock down. As the number of maintainers increase,
> the chances of someone coming back from PTO and accidentally merging
> something is high.

Agree, I would also go with a few having merge rights, to prevent above
cases.

> 
> The extraneous situation exception applies even now. I expect the
> janitors who have commit access in the event of a lock down to use their
> judgment to merge such patches.
> 
> [1]: https://mozilla-releng.net/treestatus
> 
> 
> On Thu, Aug 9, 2018 at 1:25 AM Shyam Ranganathan <srangana at redhat.com
> <mailto:srangana at redhat.com>> wrote:
> 
>     Maintainers,
> 
>     The following thread talks about a merge during a merge lockdown, with
>     differing view points (this mail is not to discuss the view points).
> 
>     The root of the problem is that we leave the current process to good
>     faith. If we have a simple rule that we will not merge anything during a
>     lock down period, this confusion and any future repetitions of the same
>     would not occur.
> 
>     I propose that we follow the simpler rule, and would like to hear
>     thoughts around this.
> 
>     This also means that in the future, we may not need to remove commit
>     access for other maintainers, as we do *not* follow a good faith policy,
>     and instead depend on being able to revert and announce on the threads
>     why we do so.
> 
>     Please note, if there are extraneous situations (say reported security
>     vulnerabilities that need fixes ASAP) we may need to loosen up the
>     stringency, as that would take precedence over the lock down. These
>     exceptions though, can be called out and hence treated as such.
> 
>     Thoughts?
> 
>     Shyam
> 
>     PS: Added Yaniv to the CC as he reported the deviance
> 
>     -------- Forwarded Message --------
>     Subject:        Re: [Gluster-devel] Release 5: Master branch health
>     report
>     (Week of 30th July)
>     Date:   Tue, 7 Aug 2018 23:22:09 +0300
>     From:   Yaniv Kaul <ykaul at redhat.com <mailto:ykaul at redhat.com>>
>     To:     Shyam Ranganathan <srangana at redhat.com
>     <mailto:srangana at redhat.com>>
>     CC:     GlusterFS Maintainers <maintainers at gluster.org
>     <mailto:maintainers at gluster.org>>, Gluster Devel
>     <gluster-devel at gluster.org <mailto:gluster-devel at gluster.org>>,
>     Nigel Babu <nigelb at redhat.com <mailto:nigelb at redhat.com>>
> 
> 
> 
> 
> 
>     On Tue, Aug 7, 2018, 10:46 PM Shyam Ranganathan <srangana at redhat.com
>     <mailto:srangana at redhat.com>
>     <mailto:srangana at redhat.com <mailto:srangana at redhat.com>>> wrote:
> 
>         On 08/07/2018 02:58 PM, Yaniv Kaul wrote:
>         >     The intention is to stabilize master and not add more patches
>         that my
>         >     destabilize it.
>         >
>         >
>         > https://review.gluster.org/#/c/20603/ has been merged.
>         > As far as I can see, it has nothing to do with stabilization and
>         should
>         > be reverted.
> 
>         Posted this on the gerrit review as well:
> 
>         <snip>
>         4.1 does not have nightly tests, those run on master only.
> 
> 
>     That should change of course. We cannot strive for stability otherwise,
>     AFAIK. 
> 
> 
>         Stability of master does not (will not), in the near term guarantee
>         stability of release branches, unless patches that impact code
>     already
>         on release branches, get fixes on master and are back ported.
> 
>         Release branches get fixes back ported (as is normal), this fix
>     and its
>         merge should not impact current master stability in any way, and
>     neither
>         stability of 4.1 branch.
>         </snip>
> 
>         The current hold is on master, not on release branches. I agree that
>         merging further code changes on release branches (for example
>     geo-rep
>         issues that are backported (see [1]), as there are tests that fail
>         regularly on master), may further destabilize the release
>     branch. This
>         patch is not one of those.
> 
> 
>     Two issues I have with the merge:
>     1. It just makes comparing master branch to release branch harder. For
>     example, to understand if there's a test that fails on master but
>     succeeds on release branch, or vice versa. 
>     2. It means we are not focused on stabilizing master branch.
>     Y.
> 
> 
>         Merging patches on release branches are allowed by release
>     owners only,
>         and usual practice is keeping the backlog low (merging weekly)
>     in these
>         cases as per the dashboard [1].
> 
>         Allowing for the above 2 reasons this patch was found,
>         - Not on master
>         - Not stabilizing or destabilizing the release branch
>         and hence was merged.
> 
>         If maintainers disagree I can revert the same.
> 
>         Shyam
> 
>         [1] Release 4.1 dashboard:
> 
>     https://review.gluster.org/#/projects/glusterfs,dashboards/dashboard:4-1-dashboard
> 
>     _______________________________________________
>     maintainers mailing list
>     maintainers at gluster.org <mailto:maintainers at gluster.org>
>     https://lists.gluster.org/mailman/listinfo/maintainers
> 
> 
> 
> -- 
> nigelb