ContentMine / meta

A repository in which to file and fix meta issues (issues affecting more than one ContentMine repo or project)
0 stars 0 forks source link

AMI stack: ensure pull requests / disable direct pushing #11

Closed ghost closed 7 years ago

ghost commented 7 years ago

@petermr (and others) historically pushed directly to repositories hosted on GitHub under the Contentmine organization (e.g. https://github.com/ContentMine/svg/ ). This works fine for solo developers, but doesn't scale so well to multi-person teams. It bypasses the code review steps that are afforded by using pull requests ("PRs"), and misses out on much of the value added by code hosting services such as GitHub, GitLab, BitBucket, etc.

Now that 3-4 developers will be working intensively on the AMI stack, @petermr and I agreed last week to disable direct pushing for the AMI stack's ContentMine repositories, in favour of PRs. This will facilitate publicly-visible code review and CI, and reduce merge conflicts.

Instead, contributors to any AMI stack repo should follow The GitHub Flow, such that each developer has:

GitHub flow diagram

(Source.)

Ideally, PRs should be performed by a contributor other than the one responsible for the PR (e.g. I would merge @petermr's PRs; he would merge mine), to ensure scrutiny; but due to the small size of the team, developers should remain able to merge their own PRs in case time pressure is high and other team members are unavailable.

This is sometimes known as a "forking workflow". It is basically the same as the "Integration-Manager Workflow" described https://git-scm.com/book/en/v2/Distributed-Git-Distributed-Workflows, except with PRs in place of the "Integration Manager".

I will put this in place as @petermr and I agreed last week, unless anyone objects.

Pinging @ContentMine/administrators for feedback. If you approve, please just "+1" or "thumbs up" this post. If you disapprove, please give constructive criticism :) Thanks!

tarrow commented 7 years ago

👍 This is a nice idea

ghost commented 7 years ago

Thanks :) I'd like to go ahead with this on Monday, if no-one objects, so that I can get on with Tilburg.

ghost commented 7 years ago

Monday has been and gone... Thumbs up so far (thanks!) from:

OK with you, @petermr ?

ghost commented 7 years ago

@petermr confirmed today in person. Will put this in place ASAP.

ghost commented 7 years ago

Am now protecting master branches for:

... done!

Please re-open this issue if any problems, and/or want to extend this to other branches besides master.

ghost commented 7 years ago

@jkbcm and @tarrow: as of this evening, after a few hours' work with me to handle some existing remotes with divergent histories, @petermr now has remotes under his namespace on GitHub for all the AMI stack modules, and will be pushing to those. Until he gets into the swing of creating feature/bugfix branches, etc, which will hopefully be in the coming weeks, his pushes may be to the master branches on those repositories.