Git branch and pull request for your first bug fix

What are a Git branch and a pull request for? A branch gives you a separate place to change code without touching the version that's running. A pull request is a proposal to merge those changes into the shared code after someone else has read them. Ever committed straight to main and then worried? Let's do it properly.

Where is the 70 đồng fix right now?

In the previous post, I wrote 11 tests with Vitest to lock in the fix for order 1044 at Mộc, a small Vietnamese coffee shop: an order of two salt coffees that added up to just 70 đồng (Vietnamese đồng; a salt coffee costs 35,000, about $1.40). The shop and its people in this series are made up, but the code and every command in this post really ran. The parsePrice function fixed the bug and the tests are green.

But all of it is still in the working folder on my machine. The shop's server still runs the old code, and the next order from the old point-of-sale (POS) machine will still have the wrong total.

The owner also had news. The shop has found Khoa, an experienced developer, who will spend one session a week reviewing code. From now on, every change needs Khoa's review before it goes to the server. So my job is to get the fix in front of Khoa in a form that's as easy to read as possible.

This is the state of my folder at the start. git status --short prints one line per file: M means a file Git already tracks has been modified, and ?? means a file Git isn't tracking yet:

 M src/orders.ts
 M src/server.ts
?? .vscode/
?? package-lock.json
?? package.json
?? src/orders.test.ts
?? wrong-tests/

The two modified files are the fix: orders.ts gains parsePrice and normalizeOrder, and server.ts calls normalizeOrder when it reads the data. package.json, package-lock.json and src/orders.test.ts are Vitest and the tests. The other two entries are leftovers. .vscode/ holds the debug configuration from the breakpoint post, and wrong-tests/ is an example of a badly written test I kept for comparison.

If you followed the earlier posts, your folder looks the same. If not, the commands in this post work on any Git repo with uncommitted changes.

What's wrong with committing straight to main?

The fastest way is three commands I'd guess everyone has typed. git add . puts everything in the folder into the commit, git commit records it, and git push sends it up. I tried the first two to see what that commit looks like:

[main ab9dc37] fix bug
 7 files changed, 1235 insertions(+), 2 deletions(-)
 create mode 100644 .vscode/launch.json
 create mode 100644 package-lock.json
 create mode 100644 package.json
 create mode 100644 src/orders.test.ts
 create mode 100644 wrong-tests/orders-copied-logic.test.ts

Look at the second line, then put yourself in Khoa's place. A commit called "fix bug", 7 files, 1,235 lines added. git show --stat shows more clearly where those lines are:

ab9dc37 fix bug
 .vscode/launch.json                     |   12 +
 package-lock.json                       | 1118 +++++++++++++++++++++++++++++++
 package.json                            |   13 +
 src/orders.test.ts                      |   54 ++
 src/orders.ts                           |   21 +
 src/server.ts                           |    7 +-
 wrong-tests/orders-copied-logic.test.ts |   12 +
 7 files changed, 1235 insertions(+), 2 deletions(-)

The actual fix is 28 lines in orders.ts and server.ts. It's buried among 1,118 lines of package-lock.json that npm generated, my personal VS Code settings, and a test file I myself labeled as wrong. server.ts also has a console.log line I added while debugging and forgot to remove, and git add . took it along.

This approach has three problems. The name "fix bug" doesn't say which bug, so three months from now nobody reading the history will know this commit is about order 1044. The commit mixes the fix with the tests and the clutter, so if the fix causes a problem and has to be undone, you undo the tests with it. And most importantly, the commit is on main. Pushing it makes it shared code immediately, before Khoa has read a single line.

Luckily I hadn't pushed. A commit that hasn't been pushed can still be changed freely. git reset HEAD~1 removes the last commit but keeps all the changes in the folder:

Unstaged changes after reset:
M	src/orders.ts
M	src/server.ts

The folder is back to where it started, and main is clean again. If you've committed to the wrong branch or want to fix your last commit, the post on Git commands to undo mistakes has a way out for each case.

How do you create a branch for the fix?

A branch in Git is just a name pointing at a commit. Creating one copies no files, so it costs almost nothing. Every commit you make afterwards goes on the new branch, while main stays put until someone merges the branch in.

`git switch -c` creates a new branch and switches to it right away:

Switched to a new branch 'fix/old-pos-prices'

Uncommitted changes come along to the new branch, so nothing is lost. I don't need to commit them or stash them first.

The branch name has two parts. The fix/ prefix says it's a bug fix; for a new feature I use feat/. The rest is a short description of the work, lowercase, joined with hyphens. Git doesn't enforce any convention, but meaningful names let others glance at the branch list and know what each branch does. Nobody dares delete a branch called test2 or my-branch, because nobody knows what's in it.

Tech Notes uses exactly this convention. The repo's guidelines say never to commit directly to main, to start every change from a feat/ or fix/ branch, and to keep each pull request to one feature or one fix.

What should you check before committing?

I have a habit of reading my own diff before committing, as if I were reviewing someone else's code. git diff with a file name shows what changed in that file since the last commit. Lines starting with - were removed, and lines starting with + were added:

diff --git a/src/server.ts b/src/server.ts
index f0f3d00..f41bc91 100644
--- a/src/server.ts
+++ b/src/server.ts
@@ -1,10 +1,13 @@
 import { readFileSync } from "node:fs";
 import { createServer } from "node:http";
 
-import { listOrders, type Order } from "./orders.ts";
+import { listOrders, normalizeOrder, type RawOrder } from "./orders.ts";
 
 const dataFile = new URL("../data/orders.json", import.meta.url);
-const orders: Order[] = JSON.parse(readFileSync(dataFile, "utf8"));
+// Normalize as soon as the file is read: from here on, prices are always numbers.
+const rawOrders: RawOrder[] = JSON.parse(readFileSync(dataFile, "utf8"));
+const orders = rawOrders.map(normalizeOrder);
+console.log("DEBUG order 1044:", orders[3]);
 
 export const server = createServer((req, res) => {
   if (req.url === "/orders") {

Look at the last + line. That's the forgotten console.log. If it reached the server, every startup would print a customer's name and phone number to the log. I deleted that line in the editor, then checked the overview again:

 src/orders.ts | 21 +++++++++++++++++++++
 src/server.ts |  6 ++++--
 2 files changed, 25 insertions(+), 2 deletions(-)

Two minutes of reading the diff let me catch this myself. If Khoa had caught it, I'd have spent a round of comments, fixing and pushing again, and he'd have spent time on something that should never have reached him.

How do you split commits so they're easy to read?

I split the changes into two commits, each doing one thing a reader can understand on its own.

The first commit is the fix. Instead of git add ., I name exactly two files:

M  src/orders.ts
M  src/server.ts
?? .vscode/
?? package-lock.json
?? package.json
?? src/orders.test.ts
?? wrong-tests/

The M is now in the first column instead of the second. The first column is the staging area, meaning what goes into the next commit. The ?? lines stay outside.

Then I run git commit without -m, so Git opens an editor and I can write a multi-line message:

fix: read "35.000" prices from the old POS machine correctly

The old point-of-sale machine writes prices as text with dots between
thousands ("35.000"). JavaScript reads the dot as a decimal point, so
the 2 salt coffees in order 1044 added up to 70 đồng instead of 70,000.

parsePrice turns prices into numbers as soon as the data is read
(normalizeOrder), and throws on unusual formats like "35,000" instead
of guessing.

The first line says what the commit does, short enough to show in full in git log --oneline. The body says why. The diff already shows how the code changed, but no diff can tell you that the old machine writes prices as "35.000", or why "35,000" is rejected. The post on writing good commit messages has more templates for other situations.

The second commit is the tests. Before committing, I run the tests once more, because I just deleted a line in server.ts:

 Test Files  1 passed (1)
      Tests  11 passed (11)

A  package-lock.json
A  package.json
A  src/orders.test.ts
?? .vscode/
?? wrong-tests/

test: add Vitest and a test that locks in the order 1044 fix

11 tests: parsePrice (5 formats it reads, 4 it rejects), order 1044,
and the totals of all four orders in data/orders.json.
Run: npm test

Why two commits rather than one? Khoa can read the first commit, 27 lines, and understand the whole fix without looking at the tests yet. package-lock.json, with over a thousand lines, sits in the second commit, where everyone knows it's generated by npm and doesn't need reading. And if I ever need to move just the fix to another branch, I can take exactly one commit.

In my view, there's no need to split further. One commit per function makes the history choppy, and readers have to piece it back together in their heads. The question I ask myself is whether I can name the commit in one sentence without the word "and".

Two ?? entries are left. The wrong-tests/ folder has done its job, so I delete it. The .vscode/ folder I want to keep on my machine but not in the repo, because it's my personal setup. .gitignore is for the whole team; for a single machine, Git has .git/info/exclude. I add a .vscode/ line there, and git status --short prints nothing.

Now let's see what this branch has that main doesn't. The syntax main..HEAD means the commits reachable from where you are but not from main:

1241165 test: add Vitest and a test that locks in the order 1044 fix
18725d4 fix: read "35.000" prices from the old POS machine correctly

These two lines already tell the story of the whole branch. Compared with a single "fix bug" line, Khoa knows what he's about to read before opening the first file.

Pushing the branch and opening a pull request

The branch still only exists on my machine. git push sends it to the shared repo, and -u origin tells Git to remember that this branch pairs with the branch of the same name on the origin repo, so next time a plain git push is enough:

Branch 'fix/old-pos-prices' set up to track remote branch 'fix/old-pos-prices' from 'origin'.
To ../moc-orders.git
 * [new branch]      fix/old-pos-prices -> fix/old-pos-prices

I ran this against a Git repo on my own machine acting as the shared repo, so the post doesn't depend on any account, and I shortened the path on the To line. When the shared repo is on GitHub, the output has a few more lines starting with remote:, including a link to open a pull request for the branch you just pushed.

A pull request, or PR, is a page on GitHub that says "I'd like to merge this branch into main, please take a look". According to GitHub's docs on creating a pull request, you pick the source and target branches, write a title and description, and create it. The page shows the whole diff and the list of commits, and lets others comment on individual lines. GitLab calls the same thing a merge request.

One thing I want to be clear about: pull requests aren't a Git feature but a feature of the code hosting platform. Git only knows branches and commits; the discussion and the Merge button belong to GitHub.

What should a pull request description say?

The PR description is the first thing a reviewer reads, before any code. I write it in four sections, and this is the description for this PR:

fix: read "35.000" prices from the old POS machine correctly

## Problem
Order 1044 (2 salt coffees) shows a total of 70 đồng instead of 70,000.
The old POS machine writes prices as the text "35.000". JavaScript reads the dot as a decimal point, so "35.000" * 2 = 70.

## Changes
- `parsePrice` turns prices into numbers as soon as the data is read, via `normalizeOrder` in `server.ts`.
- Unusual formats ("35,000", "35k", empty) throw instead of guessing.
- Adds Vitest and 11 tests, including one just for order 1044.

## How to test
1. `npm install && npm test`: 11 green tests.
2. `node src/server.ts`, open http://localhost:3000/orders: order 1044 has `"total": 70000`.

## Not in this PR
- No validation yet of order data coming from the old machine (negative quantities, missing item names).
- If `orders.json` has one badly formatted price, the server stops at startup. I'd like your opinion: should it stop, or skip that order and log it?

The "Problem" section is for someone who knows nothing about order 1044. Khoa wasn't there while I debugged, so he needs the symptom and the cause before reading the fix.

The "How to test" section is the one I see skipped most, and it saves the most time. The reviewer doesn't have to guess how to try it. They copy two commands, and if the result differs from the description, that's already a finding.

The "Not in this PR" section sounds like confessing weaknesses, but it helps in two ways. It tells the reviewer I've thought of those cases and left them on purpose, so they don't need to comment to ask. And the last line is a real question. I'm not sure whether the server should stop or skip the bad order, and asking in the description is faster than waiting to be asked.

Tech Notes works the same way. On 10 November 2026, the blog's sitemap stopped picking up new posts. The first fix went in as pull request 36 from the fix/sitemap-refresh branch. Only after deploying did it turn out not to hit the root cause, and the second fix was pull request 37 from fix/sitemap-static-path. Each PR did one thing, so when the first wasn't enough, the history still showed clearly what had been tried and why more work was needed.

Twelve comments after one evening

I opened the pull request in the afternoon. The next morning, the PR page had 12 comments from Khoa. Some were just questions, like "why does parsePrice throw instead of returning null?". Some suggested renaming things. One started with "I think there's going to be a problem here when…".

My first reaction was a small jolt of anxiety. The code works, the tests are green, and there are still twelve things to talk about. But that's exactly why the fix went through a pull request instead of straight to main: those twelve things got said before the code reached the shop's server.

Which comments to read first, how to reply, which to fix and which to discuss is the subject of the next post. You can follow the whole story in the Taking over an old project series. If you're preparing for interviews, the merge and rebase question is in the interview question bank.