14 - Git + Code Review Mechanics (Evaluator Workflow) Flashcards

PR/diff review workflow, high-signal review wording, commit hygiene, merge vs rebase, revert vs reset, and common PR risk patterns. (80 cards)

1
Q

Deck 14 - Git + Code Review Mechanics (Evaluator Workflow) (objective)

A

Goal: evaluate PRs like a senior: read diffs fast, spot risk, write high-signal review comments, and propose safe next steps.
You learn: what/why (under the hood), common traps, and evaluator-grade wording.
Output style: PASS/PARTIAL/FAIL + 2-3 issues + fix approach / review comment.

How well did you know this?
1
Not at all
2
3
4
5
Perfectly
2
Q

Mental model: what is a commit?

A

A commit is a snapshot of your repo + metadata (author, message, parent pointers).
Why: history is a graph; rewriting history changes commit IDs.

How well did you know this?
1
Not at all
2
3
4
5
Perfectly
3
Q

Mental model: branch vs commit.

A

A branch is a movable pointer to a commit.
Why: merging/rebasing moves pointers; commits are the history nodes.

How well did you know this?
1
Not at all
2
3
4
5
Perfectly
4
Q

Mental model: HEAD and detached HEAD.

A

HEAD points to your current branch (or a commit in detached mode).
Detached HEAD means commits are not on a named branch unless you create one.

How well did you know this?
1
Not at all
2
3
4
5
Perfectly
5
Q

Mental model: working tree vs index (staging).

A

Working tree: files on disk. Index: what goes into next commit.
Why: you can stage parts of files; staging defines the commit snapshot.

How well did you know this?
1
Not at all
2
3
4
5
Perfectly
6
Q

Mental model: merge vs rebase.

A

Merge keeps both histories and creates a merge commit.
Rebase replays commits onto a new base, creating new commit IDs.

How well did you know this?
1
Not at all
2
3
4
5
Perfectly
7
Q

When to prefer merge vs rebase (practical).

A

Feature branches: rebase locally to keep clean history; merge/squash when integrating.
Shared branches: avoid rebasing after push unless workflow allows it.

How well did you know this?
1
Not at all
2
3
4
5
Perfectly
8
Q

Fast-forward merge (what it is).

A

If target branch has not diverged, Git can move the branch pointer forward without a merge commit.
Why: linear history.

How well did you know this?
1
Not at all
2
3
4
5
Perfectly
9
Q

Squash merge (tradeoff).

A

Squash combines multiple commits into one.
Pros: clean history; Cons: less granular bisecting and story.

How well did you know this?
1
Not at all
2
3
4
5
Perfectly
10
Q

Cherry-pick (what/when).

A

Cherry-pick applies a single commit onto another branch.
Use for backports/isolated fixes; watch for duplicated changes.

How well did you know this?
1
Not at all
2
3
4
5
Perfectly
11
Q

Revert vs reset (critical).

A

revert creates a new commit that undoes changes (safe for shared history).
reset moves branch pointer (rewrites history) and is risky after push.

How well did you know this?
1
Not at all
2
3
4
5
Perfectly
12
Q

Force push safety rule.

A

Never force-push to shared branches unless explicitly allowed.
If needed, use –force-with-lease to avoid overwriting newer remote work.

How well did you know this?
1
Not at all
2
3
4
5
Perfectly
13
Q

Stash (what it is).

A

Stash stores uncommitted changes temporarily.
Use to switch contexts; prefer small WIP branches over long stash chains.

How well did you know this?
1
Not at all
2
3
4
5
Perfectly
14
Q

Bisect (why it matters).

A

git bisect finds the commit that introduced a bug via binary search.
Small commits and good messages make bisect effective.

How well did you know this?
1
Not at all
2
3
4
5
Perfectly
15
Q

Unified diff basics.

A

+ lines are additions, - lines are removals.
Context lines show surrounding code; reviewers focus on behavior changes.

How well did you know this?
1
Not at all
2
3
4
5
Perfectly
16
Q

Review triad: correctness, safety, maintainability.

A

Correctness: does it work? Safety: edge cases/security/races. Maintainability: can others change it safely?
Perf matters most on hotspots.

How well did you know this?
1
Not at all
2
3
4
5
Perfectly
17
Q

What to look for first in a PR (triage).

A

1) Security/data boundaries
2) Correctness/edge cases
3) Async/race/cancel
4) Maintainability
5) Performance hotspots
6) Style/nits

How well did you know this?
1
Not at all
2
3
4
5
Perfectly
18
Q

Lockfiles and dependency diffs (why noisy).

A

Lockfiles can change many lines for one package bump.
Review intent (which deps) + major version bumps + CI status.

How well did you know this?
1
Not at all
2
3
4
5
Perfectly
19
Q

Formatting churn (review rule).

A

Mass reformatting hides real changes.
Best practice: separate formatting/refactor PRs from behavior changes.

How well did you know this?
1
Not at all
2
3
4
5
Perfectly
20
Q

Code owners / required reviewers (concept).

A

Some paths require domain owners.
Why: reduces risk and enforces ownership/compliance.

How well did you know this?
1
Not at all
2
3
4
5
Perfectly
21
Q

What makes a review comment high signal?

A

1) Impact (bug/security/UX)
2) Evidence (where/why)
3) Recommendation (specific fix/options)
4) Priority (blocker vs suggestion)

How well did you know this?
1
Not at all
2
3
4
5
Perfectly
22
Q

Must-fix vs nice-to-have (rule).

A

Must-fix: correctness, security, data loss, crashes, a11y blockers.
Nice-to-have: refactors, naming, micro-optimizations, stylistic nits.

How well did you know this?
1
Not at all
2
3
4
5
Perfectly
23
Q

Blocking comment template.

A

BLOCKER: <impact>. Evidence: <where/why>. Recommendation: <specific>.</specific></impact>

How well did you know this?
1
Not at all
2
3
4
5
Perfectly
24
Q

Non-blocking suggestion template.

A

SUGGESTION: <improvement>. Rationale: <why>. Optional approach: <example>.</example></why></improvement>

How well did you know this?
1
Not at all
2
3
4
5
Perfectly
25
How to request tests (wording).
Please add coverage for: + + . Reason: prevents regressions and clarifies expected behavior.
26
Diff reading heuristic for risk.
Scan for auth, input validation, async flows, state transitions, data transforms, and removed guards. Also watch changed defaults and error handling.
27
Refactor vs behavior change (review).
Mixing refactor + feature change increases risk. Recommendation: split PRs or add tests that isolate behavior diffs.
28
Large PR strategy.
Ask for summary: intent + key files + risk areas + test plan. Review in passes: architecture, correctness, edge cases, tests, style.
29
Commit hygiene: why small commits matter.
Small commits are easier to review, revert, and bisect. They reduce hidden behavior changes.
30
Commit messages: what good looks like.
Short subject + optional body with WHY. Example: fix(auth): prevent token refresh race - brief rationale.
31
Squash vs preserve commits guidance.
Preserve commits if they tell a story and aid bisect. Squash if commits are noisy/WIP and the final change is coherent.
32
Merge conflicts: reviewer risk.
Conflict resolution can introduce subtle changes. Re-review resolved sections and ensure tests pass afterward.
33
Rebase after review (pitfall).
Rewriting history can invalidate prior review comments. If you rebase, note what changed and avoid in strict workflows.
34
Renames/moves pitfall.
Tools may show delete+add. Confirm whether change is only a move or includes hidden logic changes.
35
Generated code/snapshots review strategy.
Confirm intent, run tests, and spot-check key cases. Keep snapshots minimal; avoid reviewing huge diffs line-by-line.
36
Lockfile review strategy.
Confirm which deps changed and why (security/feature). Watch major bumps; ensure CI is green; run audit with context.
37
Config change review strategy (tsconfig/eslint).
Config changes can silently change type checking and lint behavior. Ask for rationale and verify CI/build impact.
38
Security review trigger areas.
Auth, cookies/tokens, redirects, uploads, HTML injection, CORS, user-generated content. Raise bar for validation and tests.
39
Performance review: when to block.
Block when change adds heavy work on hot paths (render loops, big lists, request storms). Otherwise prioritize correctness/clarity.
40
Accessibility review: what is blocking.
Keyboard nav breakage, missing labels, removed focus styles, incorrect roles are blockers. Minor ARIA/nits may be follow-up unless severe.
41
Command: view history graph.
git log --oneline --decorate --graph
42
Command: inspect a commit.
git show
43
Command: see current changes.
git status
44
Command: stage hunks.
git add -p
45
Command: unstage (keep work).
git reset
46
Command: discard local changes (danger).
git restore
47
Command: revert safely.
git revert
48
Command: interactive rebase (local cleanup).
git rebase -i (avoid on shared history)
49
Command: conflict resolution workflow.
Edit -> git add -> rebase --continue OR commit -> run tests
50
Command: bisect bug commit.
git bisect start (mark good/bad until culprit)
51
Command: compare branches.
git diff main..feature
52
Command: update feature with main.
git fetch origin; git rebase origin/main (or merge per workflow)
53
Command: safer force push.
git push --force-with-lease
54
Command: blame for context.
git blame
55
Command: ignore whitespace noise in diff.
git diff -w
56
Gotcha: force push to shared branch.
FAIL - Can erase teammates' work. Use feature branches; if forced, use --force-with-lease and communicate.
57
Gotcha: refactor + feature mixed in one PR.
PARTIAL - Hard to review and risky. Split PRs or add tests isolating behavior changes.
58
Gotcha: lockfile update with no explanation.
PARTIAL - Reviewer cannot confirm intent. Add summary listing upgraded deps and why.
59
Gotcha: 'fix lint' commit changes behavior.
FAIL - Lint-only commits should not change runtime behavior. Separate concerns or add tests proving behavior is unchanged.
60
Gotcha: conflict resolution hidden changes.
FAIL - Conflicts can introduce regressions. Re-review resolved sections and verify CI/tests.
61
Gotcha: vague commit message 'updates'.
PARTIAL - Low signal. Use messages describing what + why.
62
Gotcha: PR has no test plan.
PARTIAL - Add test plan (manual steps + automated tests) to build reviewer confidence.
63
Gotcha: risky change with no tests.
FAIL - For auth/data transforms/async/state changes, require tests for key cases.
64
Gotcha: approving with unresolved blockers.
FAIL - Do not approve if blockers remain. If comments are non-blocking, mark them explicitly.
65
Gotcha: rename + logic change together.
PARTIAL - Move+edit hides diffs. Split rename and logic change or provide targeted diff links.
66
Gotcha: nitpicking style over correctness.
PARTIAL - Focus on correctness/security/tests first. Style nits should not block unless they hide bugs.
67
Gotcha: requiring heavy tests for trivial change.
PARTIAL - Match rigor to risk. Avoid unnecessary process drag.
68
Gotcha: UI PR with no screenshots.
PARTIAL - Ask for before/after screenshots or a short clip to validate UX changes.
69
Gotcha: shipping debug logs.
PARTIAL - Remove or gate logs; logs can leak data and clutter observability.
70
Gotcha: dependency bump without CI green.
FAIL - Do not merge without passing CI/tests; dependency changes can break builds.
71
Mini-drill: write a blocking comment for missing tests.
BLOCKER: Change affects critical behavior but lacks test coverage. Add tests for happy path + edge case + failure case to prevent regressions.
72
Mini-drill: write a review summary (2 sentences).
Summary: . Risk: . Validation: .
73
Mini-drill: decide revert vs fix forward.
Revert when production is broken or risk is high and fix is non-trivial. Fix forward when change is correctable quickly with low risk.
74
Mini-drill: verdict on PR with 5k lines of formatting changes.
PARTIAL - Formatting churn hides behavior diffs. Split formatting-only PR from functional changes; re-run tests.
75
Mini-drill: what to ask for on a large PR.
Ask for PR summary, test plan, risk areas, and breakdown of major files. Consider splitting PR to reduce review risk.
76
Mini-drill: high-signal comment structure (example).
BLOCKER: . Evidence: . Recommendation: . (Optional) Test plan: .
77
Mini-drill: when to request a split PR.
Request split when unrelated changes (formatting/refactor/features) are combined and it increases review risk or hides behavior changes.
78
Mini-drill: how to handle review disagreements.
Clarify goals and constraints, propose alternatives, and align on must-fix vs follow-up. Escalate to owner if needed.
79
Mini-drill: reviewer wording for lockfile bump.
SUGGESTION: Please summarize which dependencies changed and why (security/feature). Also confirm CI is green and note any major version bumps.
80
Mini-drill: reviewer wording for missing test plan.
SUGGESTION: Please add a short test plan (manual steps + automated tests) so reviewers can validate expected behavior.