Put in place guidelines around last-minute changes in releases

[ctb]

During the 1.0 release process, we made a number of fairly significant changes within the last 24-48 hours before release. Some of these changes were neither tied to an issue, nor a pull request, nor reviewed, nor discussed. Acceptance testing saved our bacon here, discovering a number of problems, but some of the changes were rather arbitrary and not discussed in sufficient detail. We should think about instituting a non-release manager freeze on features and merges, as well as a 2-man rule so that even last-minute changes (which are occasionally necessary) can undergo an informal review.

@mr-c, can you write up a few short sentences on this prior to 1.1? @luizirber and @ctb to review.

[mr-c]

The minimalist policy would that "all changes to master must go through the pull-request + review"/ I would ask that the release manager or other designated individual be allowed to merge any pull request using self-review and the checklist after one or two days have passed.

[ctb]

On Sat, Apr 05, 2014 at 03:45:26PM -0700, Michael R. Crusoe wrote:

The minimalist policy would that "all changes to master must go through the pull-request + review"/ I would ask that the release manager or other designated individual be allowed to merge any pull request using self-review and the checklist after one or two days have passed.

Sounds reasonable!

[ctb]

"There will be none." ?

[ctb]

• release manager can merge after 24 hours, but use sparingly;
• otherwise, all changes must go through pull request and review, except for emergency fixes, e.g. regressions (see fix two regressions introduced overnight #490);
• changes to sandbox/ must be signed off by two people (see e.g. handle missing input files for sandbox/assemstats3.py #484, but not what I just did for updates to calc-median-distribution in sandbox/ #184...); these have no strong review criteria, apart from not breaking tests or being obviously wrong;
• no completely new PRs in the last 48 hours except for emergency fixes;

Anything else?

[ctb]

Oh, and where should these go -- dev docs?

[mr-c]

I had planned to split the docs into two parts for 1.0: User & Dev. That got pushed back but could be done for 1.1 or 1.1.2

[mr-c]

On Sun, Jun 15, 2014 at 7:45 AM, C. Titus Brown notifications@github.com
wrote:

changes to sandbox/ must be signed off by two people (see e.g. #484
#484, but not what I just did for
#184 #184...); these have no
strong review criteria, apart from not breaking tests or being obviously
wrong;

Oh, two team members. I initially read that as having a higher acceptance
bar than regular PRs.

[mr-c]

We could institute a longer RC process and disallow all but regression type fixes during that period.

[ctb]

Sorry, what does a longer RC process address? I don't feel like we've had a lot of problems? I'm loathe to have a longer period of no new PR merging.

[mr-c]

It probably isn't needed now. But if it takes a while to run future
acceptance test suites it may be useful to have a 48 hour break.

On Mon, Jun 16, 2014 at 7:03 AM, C. Titus Brown notifications@github.com
wrote:

Sorry, what does a longer RC process address? I don't feel like we've had
a lot of problems? I'm loathe to have a longer period of no new PR merging.

—
Reply to this email directly or view it on GitHub
#372 (comment).

[ctb]

I think this needs to be documented, but I'm happy with the approach we've taken in the last few releases (more people who merge/nominal signoff by someone else, release manager has emergency merge privileges but they should be used sparingly).

[ctb]

@mr-c to formalize in documentation.

[betatim]

https://github.com/dib-lab/khmer/pull/1645/files#diff-15fef81155f347109f21f0684a9c5593R52 how does this look?

[betatim]

Added to release checklist in #1645