Skip to content
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

Open
adborden opened this issue Dec 12, 2018 · 2 comments
Open

Document our code review process in our contributing docs #298

adborden opened this issue Dec 12, 2018 · 2 comments
Assignees
Labels
feature/documentation recruitment personnel and onboarding related stuff
Milestone

Comments

@adborden
Copy link
Member

No description provided.

@tdooner
Copy link
Member

tdooner commented Jan 30, 2019

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 tdooner added in progress and removed ready labels Jan 30, 2019
@adborden
Copy link
Member Author

👍 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.

@anlawyer anlawyer added the recruitment personnel and onboarding related stuff label Jan 27, 2020
@anlawyer anlawyer added this to the 2020 Ready! milestone Jan 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/documentation recruitment personnel and onboarding related stuff
Projects
None yet
Development

No branches or pull requests

4 participants