← VCE Software Development U4O2

Ineffective code review practices

Code review that is skipped, rushed, done only by the author or lacks a security checklist, so flaws get merged and reach production.

Part of: VCE Software Development - U4O2 - Cyber Security: secure software development practices — Module 44

Understanding Ineffective code review practices in VCE Software Development U4O2

What it does: A two-minute approval on 40 changed files shows a rushed review caused by time pressure, so security flaws are missed. Without a security checklist the reviewer only looks at style, so security flaws are not checked and reach production. Requiring another reviewer, often enforced by branch protection, stops authors from merging their own unchecked code. Approving without reading the changes means no real review took place. The second comment spots a specific flaw and suggests a fix. A branch rule with zero required reviewers allows author-only approval, which makes code review ineffective.

Common mistakes: The author is reviewing their own pull request, so there is no second reviewer to spot flaws they cannot see. It should be reviewer = pick_other(author). With no security checklist, the reviewer has nothing to check the changes against, so flaws like missing input validation can be missed. It should be checklist = load_checklist().

Class: _____________________ · Name: _____________________ · Date: _________

Free printable worksheet with full answer key — or add this module to a class and your students practise it interactively.

🖨 Print worksheet + answer key Add Ineffective code review practices to a class →

This module's key terms are also in a printable crossword (Modules 23–47) and word search (Modules 1–47).

Take this worksheet further — free with Cloodit

Each concept is reinforced from every angle — learned, then applied, then tested.

Learning

Duo

A developer merges their own pull request without anyone else looking at it. Which description fits best?

Ineffective code review: no second reviewer to spot flawsEffective code review: the author knows the code best

Matches

if (deadline_close) { skip_code_review(); merge(); }

↓

Skipping the code review because of the deadline lets unreviewed changes be merged, so flaws such as SQL injection can reach production. Reviews should be required for every merge.

Applying

Meanings

Term

ineffective code review

Meaning

Code review that is skipped, rushed, done only by the author or lacks a security checklist, so flaws get merged and reach production.

Practice

reviewer ← author

Testing

Gaps

IF reviewer author THEN

Check Answer

Spots

2author = pr.author

3reviewer = author

Cloodit turns curriculum-aligned coding content into journeys students work through one biome at a time.

Students earn collectable badges

Over 100 of them, across seven natural biomes.

Study Buddy

Students build a real study resource

Turns their work into revision material ready for Flash Cards and Exam study.

Diamond Dash — one of Cloodit's live, competitive Sprint games

Students lock in with Sprints

Live, competitive practice for the start of the lesson or a teacher-led lesson finisher.

A Cloodit certificate of achievement, ready to personalise and print

Custom printable certificates

Honour your top performers, or encourage a student who's trying hard — ready to print in seconds.

Add to class →

Quick Questions

1. A developer merges their own pull request without anyone else looking at it. Which description fits best?

A. Ineffective code review: no second reviewer to spot flaws ✓

B. Effective code review: the author knows the code best

The author is too close to their own work and often misses flaws. A second reviewer gives a fresh perspective and catches security problems before they are merged.

2. Which review is more likely to let a security flaw reach production?

A. A review that only checks if the code runs, with no security checklist ✓

B. A review that works through a checklist covering input validation and secrets

Without a security checklist, reviewers tend to check only that features work and miss issues such as missing input validation or hard-coded keys.

3. A deadline is tomorrow, so the team approves every pull request in under a minute. What is the problem?

A. Fast approvals prove the code is secure

B. Time pressure leads to a rushed review that misses vulnerabilities ✓

A quick approval does not mean the code is safe. Rushed reviews skip the careful reading needed to spot flaws like SQL injection.

4. Which statement about unreviewed changes is correct?

A. Merging unreviewed changes lets flaws go straight into the main branch ✓

B. Unreviewed changes are safe if the developer is experienced

Even experienced developers make mistakes. If changes are merged without review, flaws can reach production unnoticed.

5. Which is a way to fix an ineffective code review process?

A. Let the author approve their own changes to save time

B. Require a second reviewer to approve before code can be merged ✓

Requiring another reviewer, often enforced by branch protection, stops authors from merging their own unchecked code.

Match the Pairs

git merge feature-login # no review requested → An unreviewed change is merged straight into the main code, so a security flaw can reach production unnoticed.

Review status: approved by author (self-approved) → The only person who checked the code is the one who wrote it, so there is no second reviewer to spot mistakes.

Time pressure leads to a rushed review where the reviewer approves the change without reading it properly. → Review time: 2 minutes. Comment: LGTM, approved. 40 files changed

A review with no security checklist does not check for known flaws such as missing input validation or hard-coded secrets. → Checklist: none. Reviewer checks: code style only

if (deadline_close) { skip_code_review(); merge(); } → Skipping the code review because of the deadline lets unreviewed changes be merged, so flaws such as SQL injection can reach production. Reviews should be required for every merge.

required_reviewers = 0 # developers merge their own code → Setting no required reviewers means there is no second reviewer, so the author can merge their own flawed code. At least one other person should be required to approve each change.

Key Term

ineffective code review

Code review that is skipped, rushed, done only by the author or lacks a security checklist, so flaws get merged and reach production.

Fill in the Blanks

1. IF reviewer = author THEN DISPLAY "Author reviewed own code" END IF

Hint: Type the comparison operator that checks whether the reviewer and the author are the same person.

2. IF checklistUsed = FALSE THEN DISPLAY "No checklist used" END IF

Hint: Type the keyword that starts the block of code to run when the condition is true.

3. IF reviewMinutes < 5 THEN DISPLAY "Review was rushed" END IF

Hint: Type the operator that checks whether the review time is below the minimum, which signals time pressure.

4. IF approvals = 0 AND merged = TRUE THEN DISPLAY "Unreviewed change in production" END IF

Hint: Type the logical operator that requires both conditions to be true: no approvals and the change already merged.

5. reviewed ← FALSE IF reviewed = FALSE THEN DISPLAY "Review skipped"

Hint: Type the pseudocode assignment symbol that stores a value in the reviewed variable.

Spot the Error

Question 1

A pr = get_pull_request()

B author = pr.author

C reviewer = author

D reviewer.review(pr)

Line C — The author is reviewing their own pull request, so there is no second reviewer to spot flaws they cannot see. It should be reviewer = pick_other(author).

Question 2

A changes = pr.files_changed()

B checklist = None

C review(changes, checklist)

D if review.passed:

E merge(pr)

Line B — With no security checklist, the reviewer has nothing to check the changes against, so flaws like missing input validation can be missed. It should be checklist = load_checklist().

Question 3

A needed = 60 # minutes

B if deadline_is_close:

C move_deadline()

D review_time = 5

Line D — Time pressure leads to a rushed review that only skims the code. The review should be given the time it needs: review_time = needed.

Question 4

A pr = get_pull_request()

B approvals = pr.approvals

C if approvals >= 0:

D merge(pr)

Line C — With >= 0, a change is merged even when nobody has approved it, so unreviewed changes reach production. It should require at least one approval: approvals >= 1.

Question 5

A require_review = False

B min_reviewers = 2

C block_direct_push = True

Line A — Setting require_review to False lets code be merged with no review at all, so security flaws can reach production. It should be True.

Student with a class code? Join your class →