Step 8: PR Preparation

You have a validated fix on a branch, a red-before/green-after test, and a clean diff. Now you package it as a Pull Request a maintainer can review without asking you a single clarifying question. Firecracker contribution is GitHub-native and AWS-maintained: you fork, branch, push, open a PR against main, sign every commit under the DCO, add a CHANGELOG line, request reviewers, and iterate until two maintainers approve and one merges. There is no CLA, no JIRA, no mailing-list patch. This step gets the PR to the line; Step 9 runs the review.

The bar for "merged-quality" is reached here, before any maintainer looks. Review latency is outside your control; craft is not.

Goal

A DCO-signed PR against firecracker-microvm/firecracker:main with: a branch of one-logical-change commits, a CHANGELOG.md entry under the correct [Unreleased] heading, a description that links the issue and explains the why, reviewers requested, and green CI across the Buildkite matrix.

Branch, Rebase, and Final Diff

Work on a topic branch off an up-to-date main, never on main itself. Rebase onto the latest upstream so your diff is against current code and CI runs clean:

git remote -v                       # 'origin' = your fork, 'upstream' = firecracker-microvm
git fetch upstream
git rebase upstream/main            # replay your commits on current main
git diff upstream/main --stat       # final sanity: only the predicted files

Resolve any rebase conflicts now, locally, where you have context — not in the PR under review pressure. Re-run tools/devtool checkstyle and checkbuild --all after the rebase; a rebase can surface a clippy lint a neighbouring change introduced.

The CHANGELOG Entry

Every PR that changes behavior needs a CHANGELOG.md entry. Firecracker follows Keep a Changelog: one ## [Unreleased] section at the top with ### Added / Changed / Deprecated / Removed / Fixed subheadings, and each entry is a bullet that leads with the PR link:

rg -n "## \[Unreleased\]|### Added|### Changed|### Fixed" CHANGELOG.md | head

## [Unreleased]

### Fixed

- [#NNNN](https://github.com/firecracker-microvm/firecracker/pull/NNNN): Fixed
  the machine-config API accepting `vcpu_count` of 0, which deferred the failure
  to `InstanceStart` instead of rejecting it at configuration time. The value is
  now validated at the API boundary and returns a 400.

Pick the heading by user-facing effect, and be ready to defend it:

Note: You will not know the PR number until you open the PR, so add the entry with a placeholder, open the PR, then amend the placeholder to the real #NNNN and force-push. That is normal and expected — do not skip the CHANGELOG to avoid the chicken-and-egg; a missing CHANGELOG is a fast "please add a CHANGELOG entry" round-trip. Match the exact wording/format conventions of recent entries you see with the rg above; the project is picky about it.

DCO Sign-Off: Get It Right Per Commit

Firecracker requires a Developer Certificate of Origin sign-off on every commit — there is no CLA, the sign-off is the legal mechanism. The DCO bot (github.com/apps/dco) checks that each commit has a Signed-off-by: line whose name and email match the commit author. One unsigned commit fails the whole PR's DCO check.

# -s appends the Signed-off-by line to every commit you make.
git commit -s -m "Reject vcpu_count of 0 in machine config validation

The machine-config handler accepted vcpu_count == 0, which the builder
assumes to be >= 1, deferring the failure to InstanceStart instead of a
clear 400 at configuration time. Validate the lower bound at the API
boundary and return MachineConfigError::InvalidVcpuCount.

Signed-off-by: Your Name <you@example.com>"

If you already committed without -s, fix it before pushing:

git commit --amend -s            # the last commit
git rebase --signoff upstream/main   # every commit on the branch (then force-push)

The email in Signed-off-by: must be the one tied to your GitHub account, and it must match git config user.email. Get this right once, locally, before you ever push — chasing a red DCO check across force-pushes is a waste of a review cycle. See licensing-and-dco and Lab 2.2.

Commit Hygiene

Firecracker wants each commit to be a single logical change that passes the build and tests on its own. This is what makes the PR reviewable commit-by-commit and bisectable later:

• One logical change per commit. If the fix legitimately needs two independent steps (add an error variant, then use it), two commits is fine; "fix + unrelated cleanup" in one commit is not.
• The production fix and its tests can be separate commits — it lets a reviewer see the fix, then the test that proves it, and lets you demonstrate the test is red without the fix commit applied.
• Title ≤ 72 chars, imperative mood ("Reject…", not "Rejected…"/"Fixes…"). The body wraps each line at ≤72 chars and explains the why — the symptom, the violated assumption, the fix. Reviewers read the body before the diff.

git log upstream/main..HEAD --format='%s'    # each title imperative and ≤72?

Don't open a PR with a wip, fix CI, or address review commit history. Squash the noise into logical commits before you push.

Push and Open the PR

git push -u origin your-branch-name

Open the PR against main with gh (or the web UI). The description is the artifact a maintainer reads first — invest in it:

gh pr create --repo firecracker-microvm/firecracker --base main \
  --title "Reject vcpu_count of 0 in machine config validation" \
  --body-file capstone-work/pr-body.md

PR Description Template

Firecracker's PR template asks for the change, the reason, and a testing/checklist section. Fill all of it; an empty template section reads as "didn't read the contributing guide."

## Changes

Validate `vcpu_count >= 1` in the machine-config handler, returning
`MachineConfigError::InvalidVcpuCount` (HTTP 400) instead of letting `0`
flow to the builder and fail at `InstanceStart`.

## Reason

Fixes #NNNN. `firecracker.yaml` documents a minimum of 1, but the Rust path
never enforced it: `update_machine_config` accepted `0`, and the failure
surfaced later as an opaque builder error at `InstanceStart` rather than a
clear validation error at configuration time. Rejecting bad input at the API
boundary keeps internal code able to assume `vcpu_count >= 1`. This is an
original gap (bisect is good back to v1.A.0), not a regression.

## License Acceptance / Sign-off

By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license and signed off per the DCO.

## PR Checklist

- [x] Commits are signed per the DCO (`git commit -s`).
- [x] New integration test (`tests/integration_tests/functional/test_api.py`),
      red on `main`, green here; unit test in `resources.rs`.
- [x] `CHANGELOG.md` updated under `### Fixed`.
- [x] `tools/devtool checkstyle` and `checkbuild --all` pass locally.
- [x] No new attack surface; seccomp filters unchanged; no snapshot-format change.

Tip: Use a GitHub closing keyword so the issue auto-closes on merge: Fixes #NNNN / Closes #NNNN / Resolves #NNNN in the body. Reference the issue by number, link the introducing PR if it's a regression, and state the attack-surface/snapshot impact in one line each — those are the first questions a Firecracker maintainer asks.

Request Reviewers

CONTRIBUTING says to add two reviewers (a maintainer will do it for you if you don't, but doing it yourself signals you read the guide). Pick the right people, not random ones:

# Who maintains this area? Read MAINTAINERS.md and the file's recent history.
gh api repos/firecracker-microvm/firecracker/contents/MAINTAINERS.md --jq '.content' | base64 -d
git log -5 --format='%an <%ae>' -- src/vmm/src/resources.rs   # who touches this code

Request the area maintainers from MAINTAINERS.md plus, if your bug is a regression, the author of the introducing PR — they have the most context. You need ≥2 maintainer approvals to merge, and a maintainer does the merge.

What CI Runs (and How to Stay Green)

Firecracker's CI is Buildkite (driven by .buildkite/pipeline_pr.py) plus a few GitHub Actions checks. Know what runs so you can reproduce a failure locally instead of pushing blind "fix CI" commits:

Warning: CI runs both architectures. A fix that compiles and passes on x86_64 can fail on aarch64 (a different arch/ path, a different default, a feature gated by arch). If your change is anywhere near arch/, build/test the other arch before you push — discovering it from a red CI runner costs a whole review cycle. Clippy is warnings-as-errors in CI even where it isn't locally by default; checkbuild --all reproduces that.

Watch your own CI after pushing:

gh pr checks --repo firecracker-microvm/firecracker <pr-number> --watch

If a check goes red, reproduce it locally and fix it locally. Pushing speculative "maybe this fixes CI" commits is a Step-9 failure signal; the gates all have a local equivalent above.

Deliverable for Step 8

• Branch rebased onto upstream/main; git diff upstream/main --stat shows only the predicted files.
• Every commit DCO-signed (-s), one logical change each, imperative ≤72-char titles, why-focused bodies wrapped at 72.
• CHANGELOG.md entry under the correct [Unreleased] heading, leading with the PR link, matching the project's format.
• PR opened against main with a description that links the issue (Fixes #NNNN), explains the why, and states attack-surface/snapshot/perf impact.
• PR number back-filled into the CHANGELOG entry and (if used) any in-code #NNNN comment, then force-pushed.
• Two reviewers requested (area maintainers + introducing-PR author if a regression).
• CI green across DCO, style, build (both arches), and tests.

Rubric Hooks

This is the PR craft dimension (12 pts): DCO on every commit, a correct CHANGELOG entry, a description that links the issue and explains the why, clean one-logical-change commits, and green CI. It also opens Communication (10 pts): requesting the right reviewers and a description that pre-empts questions. A DCO-clean, well-described PR with a CHANGELOG and green CI scores high; a missing sign-off, an empty template, or a "fix CI" commit history scores low. See the evaluation rubric.

Validation / Self-check

Before advancing to Step 9:

1. Every commit has a Signed-off-by: line matching its author email; the DCO check is green.
2. The branch is rebased onto current upstream/main and CI is green across DCO, style, build (both arches), and tests.
3. CHANGELOG.md has an entry under the right heading, leading with the PR link, in the project's format.
4. The PR description links the issue with a closing keyword, explains the why, and names the attack-surface/snapshot/perf impact.
5. Commits are one-logical-change with imperative ≤72-char titles; there is no wip/fix CI noise in the history.
6. You requested the right reviewers and understand you need ≥2 maintainer approvals to merge.

Then go to Step 9: Engaging on GitHub & Docs.