Review and Merge Changes

Introduction

The final lesson of the version-control chapter, aligned with course 3 of the Meta Front-End Developer pathway ("Version Control"), moves from working alone to working with a team. In professional projects nobody pushes directly to the shared branch. Changes travel on a branch, are proposed in a pull request (PR), are reviewed by a colleague, are checked by automated tests, and are then merged. When two people change the same lines, Git cannot decide for them, and you must resolve a merge conflict by hand.

This lesson teaches the full collaboration loop: forking and cloning, opening a PR, reviewing and responding to feedback, keeping your branch current, resolving conflicts, choosing a merge strategy, and cleaning up. The practice scenario is original: two Harbor Workshops teammates edit the same booking function and must reconcile their changes.

Learning objectives

Describe the pull-request workflow and the purpose of code review.
Write a clear PR title and description, and keep PRs small.
Give and receive review comments constructively.
Distinguish forks, clones, branches, and remotes (origin and upstream).
Explain why conflicts happen and read conflict markers.
Resolve a conflict step by step and complete the merge.
Compare merge commit, squash merge, and rebase, and know when each is used.
Keep a branch up to date and follow the golden rule of rebasing.
Apply basic repository hygiene: branch protection, tags, and useful .gitignore and README files.

1. The collaboration workflow

A typical team loop looks like this:

Sync: update your local main (git switch main, git pull).
Branch: create a short-lived branch for one piece of work.
Commit: save small, focused commits.
Push: publish the branch (git push -u origin my-branch).
Open a pull request: ask to merge your branch into main.
Review and CI: teammates comment; automated checks (lint, tests, build) run.
Revise: push more commits to the same branch; the PR updates automatically.
Merge: when approved and green, a maintainer or you merge it.
Clean up: delete the branch and update your local main.

A pull request is a request on a hosting platform to pull the commits of one branch into another. (GitLab calls it a merge request.) It is not a Git command; it is a feature of GitHub that wraps a branch comparison with a discussion, review approvals, and status checks.

Forks and remotes

When you have write access to a repository, you push branches directly to it. For open-source projects you usually do not. You fork the repository (create your own copy on GitHub), clone your fork, and open a PR from your fork to the original.

git clone https://github.com/amira/harbor-workshops.git     # your fork
git remote add upstream https://github.com/harbor/harbor-workshops.git
git remote -v
# origin    https://github.com/amira/harbor-workshops.git (fetch/push)
# upstream  https://github.com/harbor/harbor-workshops.git (fetch/push)

git fetch upstream
git switch main
git merge upstream/main          # bring your fork's main up to date
git push origin main

By convention origin is your own copy and upstream is the original project.

2. Writing a good pull request

Reviewers are busy. A clear PR gets reviewed faster and with better feedback.

Title. Short and specific: "Warn when fewer than 3 seats remain". Not: "Fixes".

Description. Answer four questions:

What changed?
Why (link the issue or ticket)?
How can the reviewer test it (steps, screenshots for UI changes)?
What should the reviewer pay attention to (a risky area, a decision you were unsure about)?

## Summary
Show a warning on WorkshopCard when fewer than 3 seats remain.

## Why
Closes #42. Visitors were booking workshops that sold out minutes later.

## How to test
1. `npm run dev`
2. Open the Pottery workshop (capacity 5, 3 booked)
3. Confirm the "Only 2 seats left" message appears

## Notes
I used `aria-live="polite"` so screen readers announce changes.

Closes #42 in the description links the PR to the issue and closes the issue automatically when the PR is merged into the default branch.

Keep PRs small. A PR of 50 to 300 changed lines is reviewable. A PR of 3000 lines mixing a feature, a rename, and a formatting change is usually skimmed, not reviewed. Split unrelated changes into separate PRs. Opening a draft PR early signals work in progress and invites early feedback without asking for a merge.

3. Code review

Review is both quality control and knowledge sharing.

As an author:

Review your own diff before requesting review; you will catch leftover debug code.
Make sure tests and checks pass.
Respond to every comment: fix it, or explain politely why not.
Do not take comments personally. They are about the code.
Push fixes as new commits so the reviewer can see what changed; many teams squash at merge time.

As a reviewer:

Check correctness, readability, tests, accessibility, and security (for instance, user-provided text rendered unsafely).
Be specific and kind: "Could we name this seatsLeft? n is hard to follow" rather than "bad naming".
Distinguish blocking issues from suggestions ("nit:" for minor style points).
Approve, comment, or request changes using the review controls.

Automated status checks (continuous integration, CI) run linting, tests, and builds on every push. A PR with failing checks should not be merged. Branch protection rules on main can require passing checks, a minimum number of approvals, and forbid direct pushes and force-pushes.

4. Merge conflicts

Why they happen

Git merges automatically when two branches changed different parts of a file. A conflict occurs when both branches changed the same lines (or one deleted a file the other edited). Git then pauses the merge and asks you to decide.

Conflicts are normal, not a sign of failure. They are more likely when branches live a long time, when many people edit the same file, or when a formatting change touches whole files. Short branches, frequent syncs, and small PRs keep them rare.

Reading conflict markers

Suppose main and seat-warning both edited the same line of src/booking.js. After git merge main on your branch:

git merge main
# Auto-merging src/booking.js
# CONFLICT (content): Merge conflict in src/booking.js
# Automatic merge failed; fix conflicts and then commit the result.
git status
# Unmerged paths:
#   both modified:   src/booking.js

The file now contains markers:

export function canBook(seatsLeft, requested) {
<<<<<<< HEAD
  return requested > 0 && requested <= seatsLeft;
=======
  return Number.isInteger(requested) && requested >= 1 && requested <= seatsLeft;
>>>>>>> main
}

Between <<<<<<< HEAD and ======= is your version (the branch you are on).
Between ======= and >>>>>>> main is the incoming version.

Resolving, step by step

Run git status to list the conflicted files.
Open each file and decide what the final code should be. It may be one side, the other, or a combination.
Delete the markers (<<<<<<<, =======, >>>>>>>). The file must be valid code again.
Run the tests and the app to confirm the result works.
Stage the file: git add src/booking.js.
Finish: git commit (Git proposes a merge message) or git merge --continue.

To abandon the attempt and return to the state before the merge: git merge --abort.

Editors such as VS Code highlight conflicts with buttons for "Accept current", "Accept incoming", and "Accept both". They are helpful, but they only edit text. Understanding both changes is your job, and "accept both" frequently produces code that is syntactically valid but wrong.

Resolving a conflict is a programming decision. If you do not understand the other change, ask its author.

5. Merge, squash, and rebase

When a PR is ready, hosting platforms usually offer three ways to integrate it.

Strategy	What happens	History result	Good for
Merge commit	Creates a commit with two parents	Keeps every commit and the branch shape	Preserving full context
Squash and merge	Combines all PR commits into one new commit on `main`	One commit per PR, linear	Tidy history when branch commits are noisy ("wip", "fix typo")
Rebase and merge	Replays each PR commit on top of `main`	Linear, with each original commit rewritten	Linear history with meaningful individual commits

What rebase does

git rebase main takes the commits of your branch, and replays them on top of the latest main, creating new commits with new hashes.

git switch seat-warning
git fetch origin
git rebase origin/main
# resolve conflicts if any, then:
git add src/booking.js
git rebase --continue
# or: git rebase --abort
git push --force-with-lease      # needed because history was rewritten

Merge and rebase both bring in the latest changes from main. Merge preserves history exactly; rebase produces a straight line but rewrites your commits.

The golden rule of rebase: never rebase commits that other people have already based work on (shared branches such as main). Rebasing your own unshared feature branch is fine. If you must force-push a rebased personal branch, prefer --force-with-lease, which refuses to overwrite remote commits you have not seen, instead of a bare --force.

Interactive rebase (git rebase -i HEAD~3) lets you reorder, squash, or reword your own local commits before opening a PR. Use it to tidy commits before sharing, not after.

6. Hygiene and good practice

Protect main. Require PRs and passing checks; forbid force-pushes.
Name branches meaningfully: feature/seat-warning, fix/date-parse.
Delete merged branches. Most platforms offer a button; locally use git branch -d name and git fetch --prune to remove stale remote-tracking references.
Tag releases. A tag marks a version: git tag v1.0.0, git push origin v1.0.0. Semantic versioning (MAJOR.MINOR.PATCH) signals the size of changes.
Document. A README explains what the project is, how to install, run, and test it. A CONTRIBUTING.md explains the PR process.
Never commit secrets. Use .env files that are ignored and provide .env.example. Scanning tools can flag leaked keys, but prevention is better.
Use issues to track bugs and tasks, and link PRs to them.
Sync often. Pull or rebase from main at least daily on long-lived work.

7. Worked example: reconcile two edits to `canBook`

Scenario. Amira and Karim work on the Harbor Workshops booking function in src/booking.js. The original on main:

export function canBook(seatsLeft, requested) {
  return requested <= seatsLeft;
}

Amira's branch fix/negative-seats changes it to return requested > 0 && requested <= seatsLeft; and merges first.
Karim's branch fix/non-integer-seats changes the same line to return Number.isInteger(requested) && requested <= seatsLeft;.

When Karim updates his branch, a conflict appears.

Step 1: Bring in main and see the conflict.

git switch fix/non-integer-seats
git fetch origin
git merge origin/main
# CONFLICT (content): Merge conflict in src/booking.js

Step 2: Open the file.

export function canBook(seatsLeft, requested) {
<<<<<<< HEAD
  return Number.isInteger(requested) && requested <= seatsLeft;
=======
  return requested > 0 && requested <= seatsLeft;
>>>>>>> origin/main
}

Step 3: Decide. Both fixes are valid and address different problems. The right result combines the intent of both: the value must be an integer, at least 1, and not more than the seats left. Choosing only one side would reintroduce one of the two bugs.

export function canBook(seatsLeft, requested) {
  return Number.isInteger(requested) && requested >= 1 && requested <= seatsLeft;
}

Step 4: Test. Run the existing tests and add one that covers both cases:

test("rejects non-integer and non-positive requests", () => {
  expect(canBook(5, 1.5)).toBe(false);
  expect(canBook(5, 0)).toBe(false);
  expect(canBook(5, -2)).toBe(false);
  expect(canBook(5, 2)).toBe(true);
});

Step 5: Finish the merge.

git add src/booking.js src/booking.test.js
git commit            # accept the proposed merge message
git push

The PR now shows no conflicts and can be reviewed.

Why this approach and not others. Clicking "accept incoming" would have silently discarded Karim's non-integer fix. Clicking "accept both" would have left both return lines in the file, which is valid syntax but makes the second line unreachable and loses the first condition. Rebasing instead of merging would also work if the branch is private; but merging is simpler and has no force-push risk. Adding a test makes the resolution verifiable instead of a guess.

8. Comparison tables

Local and remote commands

Command	Scope	Purpose
`git switch -c name`	Local	Create and switch to a branch
`git push -u origin name`	Remote	Publish a branch and set upstream
`git fetch --prune`	Remote to local	Update remote-tracking branches and drop deleted ones
`git merge origin/main`	Local	Integrate the latest `main` into your branch
`git rebase origin/main`	Local (rewrites)	Replay your commits on the latest `main`
`git push --force-with-lease`	Remote	Safer force push after a rebase of your own branch

Fork workflow versus shared repository workflow

	Shared repository	Fork
Who	Team members with write access	Outside contributors
Where branches live	In the one shared repository	In your own copy
Remotes	`origin` only	`origin` (fork) and `upstream` (original)
PR goes	Branch to `main` in the same repo	Fork branch to original `main`

9. Pitfalls and exam tips

Pitfall: leaving markers in the file. Committing <<<<<<< lines breaks the app. Search for them before staging.
Pitfall: resolving without testing. A conflict can produce code that compiles but is wrong.
Pitfall: giant, long-lived branches. They multiply conflicts. Merge or rebase from main often.
Pitfall: rebasing a shared branch or pushing with a bare --force.
Pitfall: forgetting to update main locally after a merge, then branching from a stale base.
Pitfall: PRs that mix unrelated changes, which are hard to review and revert.
Pitfall: ignoring failed checks and asking for review anyway.
Pitfall: conflict resolution by "accept all mine" or "accept all theirs".

Exam tip. If a question asks how to bring the latest changes of main into your branch while preserving history, think git merge main. If it asks for a linear history on your private branch, think git rebase. If it mentions a branch others use, avoid rebase and force-push.

Exam tip. To cancel a conflicted merge, the answer is git merge --abort. To finish after resolving, the sequence is edit, git add, then git commit (or --continue).

Exam tip. A PR is a hosting-platform feature, not a Git command.

Mini case study: the pull request nobody could review

Situation. Noor opens a PR titled "Updates" with 2,400 changed lines: she added a new booking form, renamed 30 files, reformatted the whole codebase with a new tool, and upgraded a dependency. The description is empty. CI fails on one test. Two reviewers each leave a comment, "Too big, I cannot review this", and the PR sits for a week while main moves ahead and conflicts pile up. Noor tries to fix it by running git push --force on her branch after rebasing onto main.

Questions.

List what is wrong with the PR.
How should the work have been organized?
Is the force-push a problem here?

Solution.

It is too large and mixes four unrelated changes; the title and description tell reviewers nothing; CI is failing; and it was left open long enough to conflict with main.
Split it into separate PRs: (a) the dependency upgrade, (b) the formatter configuration and one reformatting commit done alone, (c) the file renames, and (d) the booking form with tests. Each gets a clear title, description, and test steps. Merge the mechanical changes first so the feature PR is small and its diff is readable.
A force-push to her own, unshared feature branch after a rebase is acceptable, and --force-with-lease is the safer form. It would be a problem on main or on a branch where a reviewer has pushed commits, because it could erase their work.

Key terms

Pull request (PR): a request to merge a branch, with review and checks.
Fork: your own copy of a repository on the hosting platform.
origin / upstream: conventional names for your copy and the original project.
Code review: examination of changes by teammates before merging.
CI (continuous integration): automated checks run on every push.
Conflict: overlapping changes Git cannot combine automatically.
Conflict markers: <<<<<<<, =======, >>>>>>> lines in the file.
Squash merge: combine a PR's commits into a single commit.
Rebase: replay commits on top of another base, rewriting them.
Branch protection: rules limiting direct pushes to important branches.

Summary checklist

I can describe the sync, branch, commit, push, PR, review, merge, clean-up loop.
I write PRs with a specific title, a description, and test steps, and keep them small.
I review my own diff before asking others, and respond to all comments.
I know the difference between a fork, a clone, origin, and upstream.
I can read conflict markers and resolve a conflict, test it, then commit.
I can abort a merge with git merge --abort.
I can compare merge commit, squash, and rebase.
I never rebase shared history and prefer --force-with-lease when I must force-push my own branch.
I protect main, delete merged branches, and keep secrets out of the repository.