Skip to content
FR

Version Control: Shell Commands, Git, and GitHub Reviews

Step 12 of 45

GitHub Pull Requests, Code Review, and Merge Conflicts

Introduction

GitHub pull requests and merge-conflict resolution support working with a team. This final version-control lesson aligns with Meta’s "Version Control" course. Many teams require reviewed pull requests; follow the repository’s actual policy, which may allow some direct commits. 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 overlapping edits are incompatible, Git may need a human to resolve a merge conflict. Identical changes to the same lines can merge automatically.

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

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

1. The collaboration workflow

A typical team loop looks like this:

  1. Sync: inspect status, then update local main using the team’s policy, for example git switch main and git pull --ff-only. Inspect divergence rather than overriding it.
  2. Branch: create a short-lived branch for one piece of work.
  3. Commit: save small, focused commits.
  4. Push: publish the branch (git push -u origin my-branch).
  5. Open a pull request: ask to merge your branch into main.
  6. Review and CI: teammates comment; automated checks (lint, tests, build) run.
  7. Revise: push more commits to the same branch; the PR updates automatically.
  8. Merge: when approved and green, a maintainer or you merge it.
  9. 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.

Bash
git clone https://github.com/amira/harbor-workshops.git     # illustrative fork URL
cd harbor-workshops
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 --ff-only upstream/main # update an unmodified fork main; inspect if diverged
git push origin main

Replace the illustrative URLs with real repositories. In this fork workflow, origin names your fork and upstream names the original; these are conventions, not special Git keywords.

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)?
Markdown
## Summary
Show a warning on WorkshopCard when fewer than 3 seats remain.

## Why
Closes #42. Visitors need to see low availability before choosing to book.

## 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
Check the sold-out state and how availability updates are announced to screen readers.

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. There is no official line-count limit that guarantees a good review. Aim for one coherent change, with enough context to understand it. Generated or mechanical diffs may be large, while a small but unrelated mix can still be hard to assess. 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 run when the repository’s configured events and filters trigger them; a PR does not inherently run tests on every push. Respect required checks before merging. GitHub branch protections or rulesets can require reviews and checks and restrict updates, subject to repository settings, permissions, bypass rules, and plan availability.

4. Merge conflicts

Why they happen

Git often combines compatible changes automatically. Incompatible overlapping edits, delete-versus-edit cases, and some rename situations can conflict. Different lines can also combine into logically broken code without any textual conflict, so a successful merge still needs validation.

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:

Bash
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:

JavaScript
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.

These labels describe this merge, not a universal “my code versus their code” rule. In a rebase, ours refers to the new base plus replayed work, while theirs refers to the commit being replayed. Diff3-style conflicts also include a ||||||| common-ancestor section; remove all markers and select the intended final code.

Resolving, step by step

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

To abandon an in-progress merge, use git merge --abort. Start with committed or otherwise safely preserved work: abort tries to reconstruct the pre-merge state but cannot always recover changes that were already uncommitted or were further edited during the attempt.

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

GitHub offers the following integration methods when enabled by repository settings. Its merge-commit button requests a merge commit even when a local Git merge could fast-forward.

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

Continue with Git branches and commit history for related practice.

What rebase does

A local git rebase main can replay eligible branch commits onto that base. Commits actually replayed onto a changed base get new hashes; an already-current branch can be a no-op, and equivalent commits can be omitted. main means your local reference, which may be stale; fetch before choosing origin/main.

Bash
git switch seat-warning
git fetch origin
git rebase origin/main

Only if Git pauses for a conflict, resolve the files and continue:

Bash
git add src/booking.js
git rebase --continue
# To abandon instead: git rebase --abort

For a branch that has never been published, publish normally after the rebase:

Bash
git push -u origin seat-warning

Merge and rebase both bring in the latest changes from main. Merge retains existing commits and can add a merge commit; rebase replays work onto another base and can rewrite commit identities.

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 the team permits rewriting an already-published personal branch that nobody else uses as a base, a forced update may be needed. A lease compares the remote tip with an expected value; the default --force-with-lease uses local remote-tracking information. Background fetches can update that expectation without review, so it is not proof that all remote work was seen or preserved. An explicit reviewed expected commit gives stronger control; none of these options authorizes rewriting a shared branch.

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 switch away before git branch -d name. With ordinary fetch refspecs, git fetch --prune removes stale remote-tracking references, not your local feature branches. Deletion checks ancestry rather than semantic equivalence after a squash/rebase.
  • Tag releases. A tag marks a commit; for an annotated release marker use git tag -a v1.0.0 -m "Release 1.0.0", then git push origin v1.0.0. Semantic versioning communicates compatibility under the project’s API policy, not the size of a diff. A Git tag is distinct from a hosting-platform release.
  • 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. Review incoming changes and integrate at appropriate checkpoints, using the team’s merge/rebase policy rather than an arbitrary fixed interval.

7. Worked example: reconcile two edits to canBook

Scenario. Assume seatsLeft is already validated as a non-negative safe integer. Amira and Karim work on the Harbor Workshops booking function in src/booking.js. The original on main:

JavaScript
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.

Bash
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.

JavaScript
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.

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

Continue with Jest tests and JavaScript input validation for related practice.

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

JavaScript
// src/booking.test.js, using the Jest setup from the JavaScript chapter
import { canBook } from "./booking.js";

test("rejects invalid requests and accepts the boundaries", () => {
  expect(canBook(5, 1.5)).toBe(false);
  expect(canBook(5, 0)).toBe(false);
  expect(canBook(5, -2)).toBe(false);
  expect(canBook(5, 6)).toBe(false);
  expect(canBook(5, 1)).toBe(true);
  expect(canBook(5, 5)).toBe(true);
});

Step 5: Finish the merge.

Bash
git add src/booking.js src/booking.test.js
git commit            # accept the proposed merge message
git push origin fix/non-integer-seats

The updated branch contains the fetched main history and the resolution. Re-run the configured checks and review the PR; main may have advanced again in the meantime.

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 can also work when nobody else depends on the branch’s commits; merging integrates the changes without itself requiring a forced update. 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 Conditional forced update using an expected remote tip; permission and review are still required

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 study 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: requesting final merge while required checks fail. Draft review can still help diagnose a failure before checks pass.
  • Pitfall: conflict resolution by "accept all mine" or "accept all theirs".

Study tip. To integrate fetched main history while retaining existing commits, consider git merge origin/main after fetching; git merge main only uses your local main reference. 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.

Study 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).

Study 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.

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

Solution.

  1. 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.
  2. 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.
  3. Whether rewriting the PR branch is acceptable depends on repository policy and whether anyone has pushed to it or based work on it. Coordinate first; a normal follow-up commit often preserves review context better. A lease is a conditional tip check, not a guarantee that nobody else’s work can be lost. Never override main or a shared branch merely to refresh a PR.

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 triggered by configured repository events and filters.
  • 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.

Official sources