Thuta Learning
BasicDevOps & Toolsintermediate

Code Review Done Well

What you'll walk away with

  • Explain the core ideas behind Code Review Done Well
  • Read the diagram and trace how state or data flows through the Git/GitHub workflow
  • Decide how this applies to your own project or team

Build the mental model

Once a pull request is open, someone else needs to review it, and reviewing well is its own skill.

  • Comment — feedback without a formal verdict
  • Request changes — blocks merge until addressed
  • Approve — clears the way to merge

Good review looks at more than whether the code runs.

  • Correctness — does it actually do what the description claims, including edge cases the author might have missed
  • Readability — naming, structure, and comments that explain why, not just what
  • Security implications — user input, authentication, and data handling are common places small oversights become vulnerabilities
  • Test coverage — does the change include tests for the new behavior
  • Maintainability — can the next person, who isn't the author, understand and safely extend it

Etiquette shapes whether any of this actually lands well.

Review etiquette

Comments should be specific — pointing at an exact line and a concrete suggestion — rather than vague criticism like 'this feels off.' Phrase feedback as a question or suggestion where possible, assume good intent, and separate 'this is wrong' from 'this is a style preference,' since conflating the two turns reviews adversarial when they should stay collaborative.

text
PULL REQUEST REVIEW STATES
--------------------------
                    +-------------------+
                    |   Pull Request    |
                    +-------------------+
                   /         |          \
             comment   request changes   approve
            (no verdict) (blocks merge) (clears merge)
                   \         |          /
                    +-------------------+
                    | author updates,   |
                    | reviewer re-checks|
                    +-------------------+

Connect it to a real scenario

You do not need GitHub's website to think like a reviewer — the same comparison it shows you is one git command away locally.

See what would merge

git log main..feature/add-validation lists exactly the commits a merge would bring in, nothing more — useful for sanity-checking a branch contains only what its description claims.

See the actual changes

git diff main...feature/add-validation (three dots) shows the line-by-line changes those commits introduce, compared against where the branches diverged — the same diff GitHub renders in a pull request's Files changed tab.

Reading it locally before pushing, playing reviewer on your own work first, catches an embarrassing number of issues before anyone else ever sees them: a leftover console.log, an unrelated formatting change, a variable rename that touched more files than intended.

On GitHub itself, reviewers do the same reading but with commenting built in — clicking any line opens a thread anchored to that exact line and revision, which stays visible even as the branch gets updated with new commits.

Try the working example

bash
git init -q -b main
git config user.name "Thuta Learner"
git config user.email "learner@example.com"

echo "function login() { return true; }" > login.js
git add login.js
GIT_AUTHOR_DATE="2026-01-01T09:00:00" GIT_COMMITTER_DATE="2026-01-01T09:00:00" \
  git commit -m "Add basic login function"

git checkout -b feature/add-validation
cat > login.js <<'EOF'
function login(username, password) {
  if (!username || !password) {
    return false;
  }
  return true;
}
EOF
git add login.js
GIT_AUTHOR_DATE="2026-01-01T10:00:00" GIT_COMMITTER_DATE="2026-01-01T10:00:00" \
  git commit -m "Add input validation to login"

git log main..feature/add-validation --oneline
git diff main...feature/add-validation
You should see
git log main..feature/add-validation --oneline ->
edd0062 Add input validation to login

git diff main...feature/add-validation ->
diff --git a/login.js b/login.js
index 5f8135f..7ab42fa 100644
--- a/login.js
+++ b/login.js
@@ -1 +1,6 @@
-function login() { return true; }
+function login(username, password) {
+  if (!username || !password) {
+    return false;
+  }
+  return true;
+}

5-minute try-it

In a repository with a main branch and a feature branch, run git log main..<branch> --oneline and git diff main...<branch>. Then, as a reviewer, note something for each of the five criteria: correctness, readability, security, test coverage, and maintainability.

One important caution

Leaving vague comments — criticism like 'this doesn't look right' with no specific line or suggestion leaves the author unsure what to actually do

Conflating a style preference (indentation, naming style) with a real bug, treating both as equally blocking with request changes

GitHub Docs: About pull request reviewsGit & GitHub

Easy traps

  • Leaving vague comments — criticism like 'this doesn't look right' with no specific line or suggestion leaves the author unsure what to actually do
  • Conflating a style preference (indentation, naming style) with a real bug, treating both as equally blocking with request changes
  • Always run `git status` before any destructive or history-rewriting command, to know exactly what state you're in.

Exercise

In a repository with a main branch and a feature branch, run git log main..<branch> --oneline and git diff main...<branch>. Then, as a reviewer, note something for each of the five criteria: correctness, readability, security, test coverage, and maintainability.

You'll know it worked when: git log main..feature/add-validation --oneline -> edd0062 Add input validation to login git diff main...feature/add-validation -> diff --git a/login.js b/login.js index 5f8135f..7ab42fa 100644 --- a/login.js +++ b/login.js @@ -1 +1,6 @@ -function login() { return true; } +function login(username, password) { + if (!username || !password) { + return false; + } + return true; +}

Code Review Done Well | Thuta Learning