-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Document our code review process in our contributing docs #298
Comments
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 reposIn 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-jekyllIn 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-staticIn 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. |
👍 this is great, I think this is a good enough start to submit as a PR. One suggestion from me:
|
No description provided.
The text was updated successfully, but these errors were encountered: