Pull Request Review Procedure

Suggested values: (X)=14, (N)=3 and (M)=14

0. Someone submits a change for discussion:
1. For small but clear changes, an issue is opened on GitHub describing the bug or feature.
2. For larger structural changes, an email is sent to the openFrameworks dev list before it becomes an issue.
3. If the issue is going in the wrong direction, a team member (the Core or section leaders) must comment on the issue within a certain number of days (X). If the submitter writes code before getting any feedback, they need to understand that there is no expectation that their code will be merged. So there are three possibilities:
4. there is no approval or rejection, and a PR may not be accepted further down the line
5. a team member approves the proposal, and a matching PR is eventually accepted
6. a team member rejects the proposal, and the issue is closed or rewritten until that team member is satisfied.
7. A pull request is submitted.
8. The GitHub leader (or, optionally, any team member) checks that the PR is well-formatted. If it does not fit the following criteria, they will ask that it is fixed:
9. the PR is in its own branch
10. the PR is submitted to develop
11. the PR only has commits specific to the relevant issue
12. the PR has necessary information in the description, including how it was tested and how others can test it.
13. if the PR content is such that a changelog entry is necessary, make sure any relevant information has been added to CHANGELOG.md.
14. Other team members start PR-specific discussion. Discussion at this point is comprised of:
15. reviewing the code for any obvious problems
16. providing feedback about personal experience with this topic
17. testing relevant examples on untested platforms
18. thoroughly stepping through the changes to understand potential side effects
19. examining discrepancies between the original issue(s) and the actual PR
20. @summoning any relevant team members
21. Anyone who takes time to do any of the above can offer their assessment. Note that this should not be the time to decide whether the PR is desired or not -- that should have happened earlier. The only condition where a PR should be completely rejected is when it has not gone through the entire process (e.g., someone submitted the PR without posting an issue). The assessment may be one of the following, coupled with a note about which of the above types of review were carried out (e.g., some people might just run an example, others might do more thorough testing):
22. good to merge
23. good to merge with minor changes (which are specified)
24. not good to merge without major changes (which are specified)
25. If there are more than a certain number of votes (N) saying "good to merge", with at least one of them being a thorough code review, then a Core member is @summoned to execute the merge.
26. If the PR is not merged within a certain number of days (M) without feedback from the Core, the relevant section leader may execute the merge. This can be stopped at any time by a single constructive vote against merging ("Don't merge this, until you change...") but not by a non-constructive vote ("Don't merge this, I haven't had a chance to look at it yet...").