-
Notifications
You must be signed in to change notification settings - Fork 1
Code Reviews
- Work on your own branch of business (BOB). A BOB is dedicated to implementing one feature or fixing only one bug.
- Branch off your regular branch, or if you don't have one, branch off master.
- Regularly, sync (pull master, checkout yours, merge master) your own branch with the master to prevent merge conflicts when your feature is ready.
- Document all new features: how they work, what inputs the functions accept, what the outputs are, etc.
- Test on your own branch
- Is it working? If not, fix it before going to the next step.
- Submit a pull request from your BOB to master
- Stop working on the BOB. To work on other issues while the code review is pending, create a new BOB for each issue.
- One domain lead, or two or more general developers will be checking your code.
- You cannot review your own code.
Depending on the feature, anyone on the team can claim it and become the lead code reviewer. They should assign it to themselves.
Review all of the changes line by line. If you need more context, look at the whole file. You are looking to make sure everything is:
-
well-documented
-
efficient (time, space, DB pulls / puts, CPU, etc)
-
meets code conventions
-
meets style guides
-
follows structure of the rest of the code
-
makes logical sense - if you don't understand it, it is too complicated. You have the right to ask for more documentation, recode, or reject the pull request!
-
architected logically and optimally
-
just feels right in your gut
For anything that you feel needs fixing or is unclear, leave a comment on the offending line.
Be extremely critical - the Willet codebase should be a pristine object of codified perfection! It is up to you to keep it perfect!
The person who wrote the code should address each comment either with a) A responding comment b) A code fix (commit & push)
For each new commit & push, the coder needs to add a comment to let the reviewer know there are new pending changes to review.
When the code is finally an object of pristine codified perfection, the reviewer can close and merge the code into master. Let the Git admin know so that he/she can deploy to live when it's appropriate.
- The coder should stop coding in that branch when their code review is started. Any changes they make will be added to the code review!
- Code reviews are not meant to slow things down - it's all about ensuring we have a good codebase and that everyone knows more about parts they are not working on.
- This code review process is new. Have comments, concerns, complaints, improvements?! Tell your GitHub admin!