Contributing/Merge
Proposal
This is a proposal, things may be discussed and changed.
See the forum thread: https://forums.unvanquished.net/viewtopic.php?t=2426
Contributing, Reviewing, and Merging
Here are some recommendations to better deal with contributions.
A contribution is done in three steps:
- Producing a change, see Contributing/Code and Contributing/Media;
- Submitting the change to review and reviewing it, see Contributing/Review;
- Merging the code, see Contributing/Merge.
The purpose of those proposed recommendations is to:
- Reduce the risk of pull requests being stuck in discussions or waiting for approvals to be merged.
- Reduce the risk of people being afraid of merging or approving or submitting a pull-request.
Common merge rules
As a start, some things copy pasted from Contributing/Code:
- People with commit access should always merge their own pull requests. Don't merge another contributor's code as they may have good reasons to hold back that aren't obvious.
- Get the approval of another team member before merging.
- Do not merge pull requests that have been open for less than 24 hours. Mistakes are likely if people don't get a chance to review.
Merge role hierarchy
I suggest to formally define three roles (this may not translate into GitHub user interface):
Super merger
The purpose of super merger role is to be able to take some decisions to unstuck situations.
The super merger can, as an exception, merge someone else's pull request, if needed for example if the pull request looks ready but is sleeping for too long and the submitter is not answering, or to merge some things that looks ready for a release and delaying a release is not wanted.
In all cases, those are exceptions and when going the exception way it's recommended to first write a motivated comment and to wait a delay of one day between the motivated comment and the merge.
Note
Suggested super mergers: project heads, perturbed.
Merger
Everyone with commit access is a merger, so basically the team members.
A merger can also merge a pull request from a contributor who doesn't have commit permission, or for any reason explaining why the author will not do it, the merger is then considered the reviewer and the approver.
Merging someone else's code and being the approver requires the merged code to have been written against Unvanquished source, porting a code from another software (for example porting from ioquake3 engine to Dæmon engine) requires the porting effort to be reviewed by someone else than the merger.
Any merger is an approver to someone else.
Contributors
External contributors without commit permission, but submitting pull requests.
