Making Review Easy as the Author: Small PRs, Stacked PRs, Good Descriptions and Self-Review

Version note: The examples in this post were run with .NET 10 (SDK 10.0.402), EF Core 10.0.15, xUnit.net v3 4.0.3, Angular 22.2 (TypeScript 6.0) and Git 2.54. GitHub's stacked pull requests are in public preview as of November 2026; they were not run in a GitHub repository and are described from GitHub Docs.

Guess First

A coupon feature is coming to Bookstore, the online bookstore used as the shared example across this site's series: a percentage coupon (BOOK10), a fixed-amount coupon (SAVE50), a coupon that only applies to the classics category (CLASSIC15) and a coupon box in the cart. Emre finishes the work in three days and opens a single pull request. This is what lands in front of Ayşe:

$ git log --oneline main..feature/coupons
7218f7b Coupons + small fixes

$ git diff --shortstat main...feature/coupons
 31 files changed, 2665 insertions(+), 56 deletions(-)

Guess: how many of these 2721 lines does a person really need to read? And how many of the lines that need reading are about the coupon feature? Can Ayşe review this pull request in one sitting, or will her first comment be "where do I start reading?"

How Does Pull Request Size Affect Review?

In the first post of the series we wrote "about 400 lines at most, generated files excluded" into Bookstore's review agreement. The number comes from SmartBear's study at Cisco: when more than 200 to 400 lines are reviewed at once, the share of defects found drops clearly. Google's Engineering Practices guide sets no hard limit but points the same way: a small change is reviewed faster and more carefully, it is harder to hide a bug in it, less work is thrown away if it is rejected, and it is easy to roll back. The guide also says plainly that a reviewer may turn down a change for its size alone. Line count is not the only measure: the same 200 lines are readable in one file and unreadable spread over 50 files.

Let us look at Emre's pull request with that in mind. Most of the 2665 lines were not written by a person: they are the designer file and the model snapshot EF Core generates for every migration. To show these files collapsed in GitHub's diff view, we put a .gitattributes at the repository root:

# EF Core migration designer files and the model snapshot are generated; GitHub collapses them in diffs.
src/Bookstore.Api/Data/Migrations/*.Designer.cs linguist-generated
src/Bookstore.Api/Data/Migrations/BookstoreContextModelSnapshot.cs linguist-generated

Files marked linguist-generated are collapsed by default in GitHub diffs and left out of the repository's language statistics. Locally you make the same distinction with a pathspec:

$ git diff --shortstat main...feature/coupons -- . ':(exclude)*.Designer.cs' ':(exclude)*ModelSnapshot.cs'
 28 files changed, 606 insertions(+), 56 deletions(-)

The part that needs reading is 662 lines. That is over the limit, but the real problem is not the size, it is the mix. This is the part of the git diff --stat listing that falls on Ordering and its neighbours:

$ git diff -M --stat=90 main...feature/coupons -- src/Bookstore.Api/Ordering src/Bookstore.Api/Inventory src/Bookstore.Api/Catalog README.md
 README.md                                           |  4 ++
 src/Bookstore.Api/Catalog/CatalogService.cs         |  2 +-
 src/Bookstore.Api/Inventory/InventoryService.cs     |  6 +-
 src/Bookstore.Api/Ordering/Cart.cs                  |  1 +
 src/Bookstore.Api/Ordering/CartContracts.cs         | 19 ++++++
 src/Bookstore.Api/Ordering/CartPricing.cs           | 23 ++++++++
 src/Bookstore.Api/Ordering/Coupon.cs                | 23 ++++++++
 src/Bookstore.Api/Ordering/CouponConfiguration.cs   | 31 ++++++++++
 src/Bookstore.Api/Ordering/CouponRules.cs           | 13 +++++
 src/Bookstore.Api/Ordering/CouponService.cs         | 40 +++++++++++++
 src/Bookstore.Api/Ordering/Order.cs                 |  3 +
 src/Bookstore.Api/Ordering/OrderContracts.cs        | 21 +++++++
 src/Bookstore.Api/Ordering/OrderService.cs          | 30 +++++++---
 src/Bookstore.Api/Ordering/OrderingConfiguration.cs |  4 ++
 src/Bookstore.Api/Ordering/OrderingContracts.cs     | 19 ------
 src/Bookstore.Api/Ordering/OrderingModule.cs        | 96 ++++++++++++++++++++++++-------
 16 files changed, 281 insertions(+), 54 deletions(-)

Four different kinds of change are tangled in this listing. A refactoring: cart pricing moved from OrderingModule and OrderService into CartPricing. A new data model: Coupon, its configuration and two migrations. A behaviour change: applying the coupon to the cart and the order, and a coupon box on the Angular side. And the "while I'm here" changes: OrderingContracts.cs split into two files, a variable renamed in InventoryService, a comment rewritten in CatalogService, two methods renamed. For every line, Ayşe has to ask "does this change behaviour, or did it just move?" The commit message does not help either: "Coupons + small fixes".

Splitting a Large Feature: Refactoring Apart from Behaviour

Emre rebuilds the work from scratch as reviewable parts. The rule is simple: each part does one thing, compiles on its own and passes the tests. Google's guide also requires keeping refactorings out of feature changes and, for parts that depend on each other, that none of them breaks the build.

The first part is a refactoring only. Pricing the cart lines happened with the same logic both in the cart view and when placing an order; Emre gathers it into one function:

using Bookstore.Catalog;

namespace Bookstore.Ordering;

public sealed record PricedLine(BookInfo Book, int Quantity, decimal LineTotal);

public sealed record CartPrice(IReadOnlyList<PricedLine> Lines, decimal Subtotal);

// Prices cart lines with the current catalog prices; used by the cart view and by order placement.
public static class CartPricing
{
    public static CartPrice Price(IEnumerable<CartItem> items, IReadOnlyDictionary<int, BookInfo> books)
    {
        List<PricedLine> lines =
        [
            .. items
                .Select(i => (Book: books[i.BookId], i.Quantity))
                .OrderBy(x => x.Book.Title)
                .Select(x => new PricedLine(x.Book, x.Quantity, OrderItem.CalculateLineTotal(x.Book.Price, x.Quantity)))
        ];
        return new CartPrice(lines, lines.Sum(l => l.LineTotal));
    }
}

The most important sentence in this pull request's description is "no behaviour change". The reviewer's job is to check that claim: do the existing order tests pass unchanged, do the new tests cover only the moved logic? When the refactoring comes on its own, that check is easy; buried in coupon code, it is close to impossible.

The second part is the data model: the Coupon entity, its configuration, the AddCoupons migration that adds four sample coupons, and a pure function that calculates the discount, CouponRules. When this pull request is merged no user sees anything; the table and the code are ready but no endpoint uses them. Bringing the migration in its own pull request has another benefit: the schema change can reach production before the code that uses it. Why that matters is covered in the Deployment Strategies series, in "Where Migrations Fit in a Deploy: Expand-Contract, EF Core Migration Bundles and Backward-Compatible Schemas".

The third part is the behaviour: the endpoint that applies a coupon, the discount in the cart and order views and the coupon box in Angular. This part sits behind a feature flag. The endpoint returns 404 while the flag is off, and the flag is on only in the Development environment:

// PUT /api/carts/{cartId}/coupon  { "code": "BOOK10" }
private static async Task<Results<Ok<CartView>, NotFound, ProblemHttpResult>> ApplyCoupon(
    Guid cartId, ApplyCouponRequest request, IConfiguration config,
    BookstoreContext db, ICatalogService catalog, CouponService coupons, TimeProvider clock, CancellationToken ct)
{
    if (!config.GetValue<bool>("Features:Coupons"))
    {
        return TypedResults.NotFound();
    }
    // ...
}

"Features": {
  "Coupons": true
}

The flag lets unfinished work enter main, and pull requests can be merged without waiting for the whole feature. Flag types and managing flag debt are covered in the Solution Patterns series, in "Designing Feature Flags in Code: Toggle Types, Microsoft.FeatureManagement and Flag Debt"; here a plain boolean read from configuration is enough.

What about the "while I'm here" changes? The file split and the renames were taken out of this feature. If they are really needed, they come as a separate, small pull request that does not change behaviour; if not, they do not come at all. The sizes of the three parts:

$ git diff --shortstat main...coupons/1-cart-pricing -- . ':(exclude)*.Designer.cs' ':(exclude)*ModelSnapshot.cs'
 4 files changed, 70 insertions(+), 17 deletions(-)
$ git diff --shortstat coupons/1-cart-pricing...coupons/2-coupon-model -- . ':(exclude)*.Designer.cs' ':(exclude)*ModelSnapshot.cs'
 6 files changed, 175 insertions(+)
$ git diff --shortstat coupons/2-coupon-model...coupons/3-apply-coupon -- . ':(exclude)*.Designer.cs' ':(exclude)*ModelSnapshot.cs'
 15 files changed, 332 insertions(+), 17 deletions(-)

The total is almost the same; what changed is that each pull request answers a single question. The first asks "did the price stay the same?", the second "are the model and the rules right?", the third "is the coupon applied to the cart correctly?".

Stacked Pull Requests: Building Branches on Top of Each Other

The three parts depend on each other: the second cannot be written without CartPricing, the third without Coupon. Emre does not want to sit idle waiting for the first one's review. The answer is a stack: each branch is built on the previous one, and each pull request's base is the branch below it.

$ git log --oneline --graph --decorate -4
* ec2e486 (HEAD -> coupons/3-apply-coupon) Apply coupons to carts and orders behind Features:Coupons
* c09eaa2 (coupons/2-coupon-model) Add Coupon entity, seed coupons and CouponRules
* ae17f10 (coupons/1-cart-pricing) Extract CartPricing from cart view and order placement
* 0e86d53 (main) Bookstore reference app

There are two ways to do this on GitHub. The old way is by hand: when opening the second pull request you pick coupons/1-cart-pricing as its base, so the diff shows only the second part. When the first pull request is merged and its branch is deleted, GitHub automatically retargets open pull requests based on that branch to the merged pull request's base, main. If the first pull request was squash-merged, the second branch still carries the first one's old commits and you need to rebase it onto the new main. git rebase --update-refs, which updates stacked branches in one command, is covered in the Git & GitHub series, in "Git rebase: Rewriting History and Pushing It Safely".

The new way is GitHub's own support for stacked pull requests. It opened as a public preview on 30 July 2026. According to the documentation it behaves like this (this part was not run):

• Each pull request shows only its own layer's diff; a stack map at the top shows where the layer fits.
• Every pull request in the stack is checked against the rules of the stack's base (here main), not the branch beneath it: required reviews, status checks, CODEOWNERS. Actions workflows that target main run for every layer.
• A pull request can be merged when it and every pull request below it meet the requirements. Merging the topmost ready pull request lands the unmerged layers below it in the same operation.
• When lower layers are merged, the pull requests above stay open and are rebased and retargeted automatically. Approvals survive the rebase if the diff is unchanged.
• Limits: all branches must be in the same repository, stacks cannot come from forks; GitHub Desktop does not support them; bypass permissions work only on the bottom pull request. The announcement said merge queue support was rolling out gradually.

The command-line side is a GitHub CLI extension. Installing it and the commands named in the documentation:

gh extension install github/gh-stack
gh stack init --base main
gh stack rebase
gh stack push

The main gain of a native stack is on the reviewer's side: whichever layer Ayşe opens, she sees only that layer, and she can approve the layers separately. A stack has a cost too: when something changes in a lower layer, every layer above is rebased again. The higher the stack, the higher that cost; a stack of more than three or four layers usually means the work was sliced the wrong way.

A Good Pull Request Description

Even a small diff raises questions when read without context. Bookstore's pull request template lives in .github/pull_request_template.md; GitHub fills the description box with it when a new pull request is opened:

## Why

<!-- The problem or story this PR solves. Link the issue. -->

## What changed

<!-- The behaviour change in two or three sentences. Say what is intentionally left out. -->

## How it was tested

<!-- Commands you ran, tests you added, manual checks. -->

## Screenshots

<!-- UI changes only: before and after. Delete this section otherwise. -->

## Risk and rollback

<!-- Migrations, feature flags, configuration. How do we undo this if it goes wrong? -->

## Notes for reviewers

<!-- Where to start reading, what to skip, PRs below or above this one in a stack. -->

Each heading of the template answers a question the reviewer would ask. The description Emre wrote for the third pull request:

## Why

Customers can enter a coupon code in the cart (story BS-214). Selin wants BOOK10, SAVE50,
CLASSIC15 and WELCOME25 for the December campaign.

## What changed

- PUT and DELETE /api/carts/{cartId}/coupon apply or remove a coupon.
- The cart view and the order now have Subtotal, Discount and CouponCode.
- Coupon box on the cart page.
- Not in this PR: usage limits per customer, coupons for institutional orders.

## How it was tested

- dotnet test: 17 passed (2 new tests in CouponTests).
- npm test in web/bookstore-web: passed.
- Manually: BOOK10 and CLASSIC15 on a cart with Dune and Emma.

## Risk and rollback

- Behind Features:Coupons, true only in appsettings.Development.json.
- Migration ApplyCoupons adds nullable CouponCode to Carts and Orders, Subtotal and Discount to Orders.
  Rollback: switch the flag off; the columns can stay.

## Notes for reviewers

Stack: 1/3 CartPricing refactor, 2/3 coupon model, this is 3/3.
Start with CouponService.cs and OrderingModule.ApplyCoupon. The migration Designer file is generated.

The "Not in this PR" line is small but valuable: it saves the reviewer from writing "where is the usage limit?" and draws the scope clearly. "How it was tested" contains the command and its result, not the sentence "I tested it". "Notes for reviewers" gives the reading order; in the third post we will see how the reviewer uses it.

Self-Review and When to Ask for Review

Emre first opens the pull request as a draft. CI runs on a draft pull request, but it cannot be merged and no review request goes to the owners in CODEOWNERS; that waits until the pull request is marked "ready for review". Meanwhile Emre reads his own diff on the screen Ayşe will see, with her eyes. Reading in your own editor is different from reading in GitHub's diff view; in the latter, a forgotten debug line, an accidentally committed file or an unrelated change stands out much more.

The checks before asking for review are short:

$ git diff --check main...coupons/3-apply-coupon
$ dotnet test
Test run summary: Passed!
  total: 17
  failed: 0
  succeeded: 17
  skipped: 0
  duration: 2s 048ms
$ cd web/bookstore-web && npm test -- --watch=false
 Test Files  1 passed (1)
      Tests  1 passed (1)

When git diff --check prints nothing, no trailing whitespace or conflict markers are left. The tests pass locally. Still, "ready for review" is clicked after CI turns green; asking for review with a red CI spends the reviewer's time on an unsolved problem.

The real gain of self-review is asking questions about your own code. If the answer to "why did I write this line?" is not clear from the code, a name should change, a comment should be added, or the answer should go into the description. If you do not ask that question, the reviewer will, and a round is lost.

Review Rounds: Updating Without Losing the Comments

Ayşe leaves a comment on the first pull request: CartPricing rounds each line total separately, a business rule discussed in the Requirements Analysis series, but no test protects it. Emre writes the test at the top of the stack as a fixup commit aimed at the first commit, and rebuilds the stack in one command:

$ git commit --quiet --fixup=ae17f10
$ git rebase -i --autosquash --update-refs main
Successfully rebased and updated refs/heads/coupons/3-apply-coupon.
Updated the following refs with --update-refs:
	refs/heads/coupons/1-cart-pricing
	refs/heads/coupons/2-coupon-model
$ git log --oneline --graph --decorate -4
* ce9a98f (HEAD -> coupons/3-apply-coupon) Apply coupons to carts and orders behind Features:Coupons
* 3c3a74b (coupons/2-coupon-model) Add Coupon entity, seed coupons and CouponRules
* 05f87fe (coupons/1-cart-pricing) Extract CartPricing from cart view and order placement
* c139989 (main) Add pull request template, mark EF Core generated files

The fixup commit was folded into the first pull request's commit and all three branches moved to new commits. In the meantime Ayşe had merged the pull request template into main with a separate small pull request; the stack moved on top of that as well. Updating the three branches on the remote now needs a force push; why to do it with --force-with-lease is in the Git & GitHub series' rebase post. The cleanest way to show the reviewer what changed is git range-diff: it compares two commit ranges and shows only the difference:

$ git range-diff 0e86d53..pr1-round1 main..coupons/1-cart-pricing
1:  ae17f10 ! 1:  05f87fe Extract CartPricing from cart view and order placement
    @@ tests/Bookstore.Api.Tests/CartPricingTests.cs (new)
     +        Assert.Empty(price.Lines);
     +        Assert.Equal(0m, price.Subtotal);
     +    }
    ++
    ++    [Fact]
    ++    public void Each_line_is_rounded_before_the_subtotal_is_summed()
    ++    {
    ++        Dictionary<int, BookInfo> books = new()
    ++        {
    ++            [1] = new BookInfo(1, "9780141439518", "Pride and Prejudice", 0.125m),
    ++            [2] = new BookInfo(2, "9780141439587", "Emma", 0.125m)
    ++        };
    ++
    ++        CartPrice price = CartPricing.Price(
    ++            [new CartItem { BookId = 1, Quantity = 1 }, new CartItem { BookId = 2, Quantity = 1 }], books);
    ++
    ++        // 0.125 is rounded to 0.13 on each line; rounding only the sum would give 0.25.
    ++        Assert.Equal(0.26m, price.Subtotal);
    ++    }
     +}

pr1-round1 is a tag we put on the commit before the review; since main moved in between, the two ranges have different bases, but range-diff does not mind and shows only the added test. Lines starting with a double ++ mean "added in the new diff, not present in the old one".

There is a trade-off here. A force push makes comments attached to old commits "outdated" on GitHub and weakens the reviewer's "changes since your last review" view. Without a stack, the plainest path is not rewriting history during review: add fixes as new commits and close the pull request with a squash merge. With a stack, a change in a lower layer needs a rebase anyway; then posting the range-diff output as a comment, or using the native stack's "Rebase stack" button, keeps the context. Interactive rebase and squash options are compared in the Git & GitHub series, in "Git Interactive Rebase and Squash: Tidying History Before a PR".

In Production

The layers of a stack can go to production separately; that is good, but the order has to make sense. The AddCoupons migration is in the second layer; when that layer is merged and deployed, a new table appears in the database and no code uses it. The third layer's migration, however, adds columns to the existing Orders table. Changes like that must be safe to apply while the old version is still running; in the third post we will see a problem with this migration that the review caught.

The feature flag needs an end date too. If, after the coupon feature is switched on in production and runs without problems for a while, nobody opens the pull request that deletes the flag and the if checking it, the flag turns into permanent configuration. The "Risk and rollback" section of the description is the place for that cleanup as well: writing down when the flag goes away makes it harder to forget.

And measurement. Pull request size and time to first review show whether the team has really picked up the habits in this post. In the fifth post we will write a small tool that gets these numbers from GitHub data.

Common Pitfalls

A big pull request titled "small fixes". The opening pull request was titled "Coupons + small fixes", and the small fixes were exactly what made the real change hard to read. If a pull request title contains "and" or "+", it is probably two pull requests.

Force-pushing during review without a word. If the branch history changes while the reviewer is writing comments, the comments stay attached to old lines and the question "what was this answering?" comes up. If a rebase is needed, write a sentence on the pull request and add the range-diff.

Splitting by file type instead of by layer. Pull requests split as "first all entities, then all services, then all controllers" make no sense on their own and often do not compile on their own. Each layer should be a working, tested step.

Repeating the diff in the description. "Added CouponService.cs, changed Order.cs" tells the reviewer what they already see. The description's job is to tell what the diff does not show: why, what was left out on purpose, how it was tested.

Overlooking generated files, or the opposite, using them as an excuse. Migration designer files are not read, but the migration itself (Up and Down) always is. "Most lines are generated" does not let the non-generated part skip review.

A Small Experiment

★☆☆ In your own repository, run git diff --shortstat for the last five merged pull requests, excluding generated files. Which are over 400 lines? Do their titles contain "and"?

★★☆ Add a pull request template to your repository and open your next pull request with it. If writing the "Not in this PR" line was hard, the scope is not clear yet.

★★★ Build a small three-commit stack (refactoring, model, behaviour), write a fixup for the bottom commit and update the stack with git rebase -i --autosquash --update-refs main. Then produce the difference you would show the reviewer with git range-diff, and check that you can explain what changed by reading the output yourself.

Emre split his work into reviewable parts and wrote a description for each. Now it is Ayşe's turn. In the next post we move to the reviewer's seat: in which order to read a pull request, what to look for in .NET and Angular code, and what the review actually found in the third layer of the coupon pull request.