Lesson 52 · Engineering practice

Code review

Before a change joins the shared code, another engineer reads it. Follow one small pull request: a six-line Python function with a hidden bug that its own tests miss. See what a reviewer looks for, how a kind and specific comment attaches to a line, how the author answers with an update, and how an approval and green checks open the way to merge.

10 min10 steps5 questionsBeginner

Fig 52 · Interactive

Loading figure…

or ← →

Check yourself

5 quick questions

Pick an answer to see why. Your score saves with your progress.

01What is a pull request?

A pull request (PR) asks to merge your branch into main. It shows the diff, the lines you added or removed, and collects the review comments and check results.

02In the lesson, CI was green, yet the code had a bug. Why?

Checks only prove what the tests ask. No test tried 50, so 50 > 50 being False went unnoticed until a person read the code and asked for that test.

03Which review comment is the most helpful?

A good comment is kind and specific: it is about the code, not the person, gives the reason and suggests a fix.

04Why do teams prefer small, focused pull requests?

One idea per PR keeps reviews quick and careful. Google's guide calls about 100 lines usually reasonable and 1,000 lines usually too large.

05With a protected main branch, when can this pull request merge?

Branch protection rules make merging wait for the required approving reviews and passing status checks.

Key terms

Glossary

Code review: Another engineer reads a change before it merges, to catch bugs, share knowledge and keep the code consistent.
Pull request (PR): A request to merge a branch into the shared code. It shows the diff and holds the review discussion.
Diff: The difference between two versions of the code: the lines added (+) and removed (-).
Reviewer: The engineer who reads the change and comments, requests changes or approves.
Request changes: A review decision that flags feedback the author should address before merging.
Approve: A review decision that says the change is ready to merge.
CI checks: Automated checks, such as tests, that run on every push to the pull request.
Branch protection: A repository rule, for example: main only accepts pull requests with an approval and passing checks.

Fine print

Notes and sources

Google's guide is a rule of thumb, not a law: it says the right size is up to the reviewer's judgment. A PR that renames one thing across 500 files can be easy to review.
The tests sit next to the function here to keep the picture small. Real projects usually keep tests in their own files, such as test_scores.py, and run them with pytest.
Google's review guide lists more: design, functionality, complexity, tests, naming, comments, style and documentation. Security reviews often follow checklists such as the OWASP Code Review Guide.
Which approvals and checks are required is a repository setting (on GitHub, a branch protection rule). Many teams require one approval; some require two, or a code owner.
A study of code review at Microsoft (Bacchelli and Bird, 2013) found that finding defects was the top reason people gave for reviewing, but sharing knowledge and team awareness were just as important outcomes.

Quiz Unlocks after the last step.