Our pr-approval-merge conventions: permission vs forgiveness

[rmkaplan]

The convention of submitting a pr and then waiting for an independent approval before merging is perhaps not well-tuned to our current development situation.

We don't have lots of programmers making changes in the core Medley code, and we don't have a phalanx of people to do the safeguarding that comes from independent approvals. (Also, our current "product" is not being used to control nuclear power plants.)

The consequence is that pr's can languish for quite awhile (for weeks if not months), before the very few of us have time or inclination to inspect and approve. And a side-effect of that is that we end up with a tree of pr's, with inherited dependencies, as development continues along one or more paths. Which makes the approval even more complicated.

So I propose changing our approval/merging policy so that changes don't get hung up for indefinite periods of time. When a pr is submitted, that starts a clock running to give time for independent inspection, comments, and even old-style approvals. The clock runs for, say, 7 days, and if no serious problems have been brought up during that period, then the pr is available for merging, even by its original author.

If nobody looks at it during the week, it may be that a serious problem (or an even better implementation) will be missed. But if issue emerges later, with git we can always back up and redo.

In sum, in our situation and with our people and resources, I think we should reduce the overhead and delay that comes from asking for permission in advance of doing a merge, and instead ask for forgiveness after the fact, if we screw things up. With a week in the middle to minimize the risk.

[rmkaplan]

There was general agreement that this would be a reasonable way to proceed, with the modification that the default approval be allowed after 2 weeks have passed, not just one week. This ensures that there is at least one meeting in which any controversies can be discussed.

[rmkaplan]

A somewhat related issue: Do we have a policy for deleting branches that have been previously merged? Many branches still show up in the git desktop that are no longer active.

Also, the git desktop also shows old branches that have not been merged, but for which there is no open pr (and perhaps no pr was ever issued). Presumably those branches were candidates for merging that were withdrawn or superseded by branches with a better set of commits.

These also show up in the GITFNS menu (for the change-of-branch command), which offers all unmerged current branches. This is a little confusing.

When is it safe and appropriate to delete branches?

[nbriggs]

For PR branches I think it's up to the creator of the branch to delete it once it has been merged.

You can clean up your local tracking branches (if you've got local checkouts of remote branches that have since been deleted) with git remote prune origin (but test beforehand to see what it would do with git remote prune -n origin)

[nbriggs]

On github, you can visit https://github.com/Interlisp/medley/branches/yours and for the branches that have been merged you could consider deleting them.

[masinter]

I've been periodically cleaning out old branches based on whether they've been merged or closed without merging, a while ago, and (if closed) I knew why I closed it.

I think deleting old branches for which there isn't a PR probably requires some investigation.

It doesn't really hurt to keep them around since the incremental storage is small (git keeps the diffs, not a copy of the whole branch).

[masinter]

I liked the idea I heard as far as approving old unmerged (non-draft) PRs that the requestor should bring it up in a meeting and we schedule time to discuss.

That seemed better than an arbitrary timeout.
So the normal "round-table" should include, In addition to "status & plans", add "blockers" (things you're waiting on someone else to do).
Would that help?
Although we don't have a lot of people now, I want to set up our processes so they can accommodate more contributors; lack of clear processes is a inhibitor.

[masinter]

Include in the website under "getting involved"?