[[
wikihub
]]
Search
⌘K
Explore
People
For Agents
Sign in
Explore
People
For Agents
Sign in
@jemoka / Jemoka Knowledge Base / raw/concept/kbhcode_review.md
Suggest edit
Cancel
Submit suggestion
Title
Name
Note
--- title: "Code Review" source: https://www.jemoka.com/posts/kbhcode_review/ --- A Code Review is a systematic study code by others—like proofreading an essay. There’s a few different ways of doing Code Review. Why do we code review? catch bugs, style deviations, design + convention violations security trade-off: having someone who is well-versed in security is useful to know how other people’s code work to learn additional skills, languages, frameworks Code Review Methodology Don’t do it Very fast! None of the benefits of code review Over-the-Shoulder Code Review Over-the-Shoulder Code Review typically is done over someone’s shoulder—author walking the reviewer through code. Props Typically catch major + obvious issues Lightweight and fast—most likely to get done Cons Author explains code as they go; biasing the reviewer Author knows the code, so may gloss over the parts they are familiar with + move at a pace faster than the reviewer can process The author only has two solders: can’t involve more than 2 people Usually real-time: the author is waiting for reviewer, which is blocking both author and reviewer’s schedules Pair Programming Pair Programming is a two-brains, one keyboard paradigm. The less experienced person types (moves at the pace of the slower person) Pros Real-time feedback/correction; good for learning new things Writing code + code review at the same time—total time saved Cons Two people working together are susceptible to bind slots still! Remember that it will take four times as long to do something: this trade-off is worth it! Formal Code Review Tools: Phabricator, Gerrit The process of Formal Code Review is a very formal process. Author writes and commits code The diff of the commit is sent to the reviewer Reviewer reads through the code at their own pace Reviewer can comment on the entire diff, or on specific lines of code This can involve multiple reviewers This is basically informal code review, but solving the original issues. Best Practices Every commit must be code reviewed before pushed to other people The larger the commit, the more reviewers should be on it Reviewer Approval Levels +2: code can be pushed +1: code looks good, someone else should make the call 0: why am I here? I dunno -1: this code smells funny, but I’m willing to be overruled -2: this code must be changed before being pushed -1/-2: comment should clearly indicate whether they are blocking push.