Github pull request handling for the EPICS V4 Working Group

pull-request-guidelines.md

Github pull request handling for the EPICS V4 Working Group

• Module owners may commit and push minor changes directly to the master branch of their own modules on github.
• For major changes that should be tested and reviewed by other people, module owners should work in a private repository and create a pull request when the work is ready for review.
• Anybody may create or comment on a pull request on any module -- this includes unsolicited contributions from non-members.
• A module's owner normally makes the final decision on merging pull requests for their modules and performs the merge (in many cases this just means pushing the green Merge button on the pull request webpage and confirming the merge).
• A meeting of the working group may nominate someone other than a module's owner to perform merge operations for specific changes discussed by the group.
• Large or API-changing modifications should never be merged without allowing time for discussion, which should happen through the github comment facility on the specific pull request, not on the pvdata-devel mailing list.
• Minimum discussion periods for pull requests may vary in length depending on the size and significance of the change being proposed, and the amount of discussion that actually takes place. A module owner should not accept a pull request just to terminate ongoing discussion about it.
• Our group members are spread out over many different countries and timezones. Everyone should get at minimum a full working day to respond to a pull request, allowing for weekends and local public holidays.
• Group members who review a pull request for someone else's module may add a comment to that effect, e.g. "Review: Approve" or "Review: OK".
• Disapproval comments deserve more discussion, but if you don't have time to write them immediately, something like "Review: Hold" should warn the module owner to await further feedback before merging.

Draft 2: ANJ 2015-08-21

Re: Non-contentious minor changes may be merged immediately, but the whole group may not have time to look at the request in less than a week or more.

1. I don't really understand this sentence.
2. I fear that this is too fuzzy and can easily be stretched to allow anyone anything.

I would try to be clearer: Pull requests shall be reviewed.
Non-contentious minor changes can be merged directly (see first bullet) and do not need to (actually should not) be created as pull requests in the first place.

Minor changes by non-members obviously have to be pull requests, but a working group member will have a look at the changes before merging, which IMHO is a review.

I would like to have a minimum wait period for pull requests. Mainly to avoid repetitive discussions if a specific merge was done too early or not.

Effectively, each working group member should have at least one full work day to look at a request before it is merged. Honoring weekends, time zones and public holidays.
Is there a compact way to state this?

Do these changes make the meaning clearer?