-
Notifications
You must be signed in to change notification settings - Fork 1
Code Reviews
Just finished a brand new feature? Have you tested it a) in all browsers? b) all edges cases you can think of? If it's all working, let's start to merge it into master so others can test it too!
When you're ready, issue a GitHub Pull Request from your personal branch into master. Once you've done that, sit back and wait for the comments to come in!
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, there's something wrong with it!
-
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.