odca-jekyll icon indicating copy to clipboard operation
odca-jekyll copied to clipboard

Document our code review process in our contributing docs

Open adborden opened this issue 6 years ago • 2 comments

adborden avatar Dec 12 '18 05:12 adborden

In order to do this, we should probably actually define the process. Here's my straw man, please suggest changes either in subsequent comments or by editing this one:

both repos

In general, authors of code should merge their own code. However, if the change is exceptionally small, or the author is not available, it is acceptable for someone else to take ownership over the change and merge the request. Whoever merges the change should ensure that the changes work in production.

odca-jekyll

In the frontend repo, code must be reviewed by at least one other person with context on the application. This currently is Aaron, Colin, Alison, Maddy, and Jesse.

When pushing a PR, please assign it to somebody to do the review. That person should either review it within a week or opt-out.

Since Github notifications can sometimes get lost in the shuffle, we encourage you to ping the reviewer on Slack after a couple days if you haven't heard from them.

disclosure-backend-static

In the backend repo, code should be reviewed by Tom or Mike. All significant functionality changes must be reviewed, but simple bug fixes do not need to be reviewed. Either way, the committer should create a PR and tag Tom or Mike so the notification is generated.

Feel free to ping Tom and Mike in Slack to look at code changes. They are usually responsive, especially Mike.

tdooner avatar Jan 30 '19 03:01 tdooner

:+1: this is great, I think this is a good enough start to submit as a PR. One suggestion from me:

In the frontend repo, code must be reviewed by at least one ~other person with context on the application. This currently is Aaron, Colin, Alison, Maddy, and Jesse~ person from the @caciviclab/disclosure team.

adborden avatar Jan 30 '19 06:01 adborden