Step 8: Pull Request Preparation

The fix is correct, tested, and validated. Now you package it as a Pull Request that a maintainer can review without friction. OpenSearch has no JIRA and no CLA — contribution flows entirely through a GitHub PR with a DCO sign-off, a CHANGELOG entry, and the PR template filled out honestly. Getting these mechanics right is the difference between "reviewed today" and "sat in the queue because the DCO check is red."

The rule that governs this step: the PR should answer every question a reviewer would ask before they have to ask it — problem, root cause, fix, tests, BWC.

Fork and Branch

You don't push to opensearch-project/OpenSearch; you push to your fork and open a PR from it.

# One-time: fork on GitHub, then add it as a remote.
gh repo fork opensearch-project/OpenSearch --remote --clone=false
# (or manually:) git remote add fork git@github.com:<you>/OpenSearch.git

# Branch off an up-to-date main with a descriptive name.
git fetch origin
git checkout -b fix/NNNN-terms-missing-min-doc-count origin/main

Branch naming: fix/<issue>-<slug> or feature/<slug>. The issue number in the branch name is a small kindness — it threads the work together.

Commit with DCO Sign-off (git commit -s)

Every commit must carry a Signed-off-by: trailer. This is the Developer Certificate of Origin — your attestation that you wrote the code and can license it Apache-2.0. The -s flag adds it:

git add server/src/main/java/org/opensearch/.../TermsAggregator.java \
        server/src/test/java/org/opensearch/.../TermsAggregatorTests.java \
        rest-api-spec/.../terms_missing_min_doc_count.yml \
        CHANGELOG.md
git commit -s -m "Fix terms agg dropping missing bucket with min_doc_count=0 on empty shards"

The commit message ends up with:

Fix terms agg dropping missing bucket with min_doc_count=0 on empty shards

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

Warning: The DCO check is a required status check. If any commit lacks Signed-off-by:, the check goes red and the PR cannot merge. The Signed-off-by: name/email must match your git config user.name/user.email. If you forgot -s, fix it without rewriting unrelated history:

git commit --amend -s --no-edit            # last commit
git rebase --signoff origin/main           # all commits on the branch

Keep the commit message subject in the imperative, under ~72 chars, and let the body (if any) explain why, not what (the diff shows what).

The CHANGELOG Entry

OpenSearch tracks a human-readable CHANGELOG.md. Every PR adds exactly one line under the right heading in the [Unreleased ...] section. Skipping it fails the changelog check.

grep -n "## \[Unreleased" CHANGELOG.md      # find the unreleased section + subheadings

The subheadings follow Keep-a-Changelog: Added, Changed, Deprecated, Removed, Fixed, Security. A bug fix goes under Fixed:

 ### Fixed
+- Fix `terms` aggregation dropping the `missing` bucket when `min_doc_count` is 0 and a shard has no matching documents ([#NNNN](https://github.com/opensearch-project/OpenSearch/pull/NNNN))

The entry is one line, user-facing (describe the behavior, not the code), backtick the API surface, and link the PR number (you'll update the placeholder to the real PR number after opening it — or use the issue number and let the bot help). A behavior change that isn't strictly a bug goes under Changed; a new capability under Added. See Step 9 for choosing between Changed and Fixed.

The CHANGELOG conflict on rebase (and how to fix it)

CHANGELOG.md is the one file every open PR edits, and they all append to the same ### Fixed block. So the moment two PRs touch it, whichever merges second must rebase — and the rebase conflicts on your one line:

$ git fetch origin && git rebase origin/main
Auto-merging CHANGELOG.md
CONFLICT (content): Merge conflict in CHANGELOG.md
error: could not apply 1a2b3c4... Fix terms agg dropping missing bucket...

This is the easiest conflict you will ever resolve, because it is not a real semantic conflict — it is two independent lines that Git could not order. Open the file, and you'll see both entries wrapped in conflict markers:

 ### Fixed
<<<<<<< HEAD
- Fix segment replication stats race on relocating shards ([#NNNN](...))
=======
- Fix `terms` aggregation dropping the `missing` bucket when `min_doc_count` is 0 ([#MMMM](...))
>>>>>>> 1a2b3c4 (Fix terms agg dropping missing bucket...)

The fix is to keep both lines — delete only the three marker lines (<<<<<<<, =======, >>>>>>>) and leave your entry and theirs:

# after editing CHANGELOG.md to keep both entries:
git add CHANGELOG.md
git rebase --continue

Never "resolve" it by dropping the other PR's line (you'd wipe their entry) or by running git checkout --ours/--theirs (that keeps exactly one). It is an append: both lines survive.

Note: Verify the CHANGELOG convention against your checkout before you rely on it. As of the 3.6 release the OpenSearch main-branch CHANGELOG.md was retired for release-note generation (see the top of CHANGELOG.md and issue #21071); PRs that intentionally skip it carry a skip-changelog label instead. On lines that still keep a CHANGELOG (older maintenance branches, and any repo that kept the check), the append-and-rebase conflict above is a near-certainty on a busy week — which is why the fix is muscle memory, not a puzzle.

The PR Template

.github/pull_request_template.md auto-populates the PR body. Fill every section — empty checkboxes and unanswered prompts read as "didn't bother." The template asks for:

  • Description — what and why.
  • Related Issues — Resolves #NNNN (or Fixes #NNNN), which auto-closes the issue on merge.
  • Check List — DCO sign-off, CHANGELOG updated, tests added, commit messages follow guidelines, public-API/BWC considered. Tick them honestly; if one doesn't apply, say why rather than leaving it blank.

A Model PR Description

This is what reviewers love to open: it front-loads the problem, the proven root cause, the scoped fix, the tests, and the BWC stance — exactly the artifacts you built in Steps 2–7.

### Description

`terms` aggregations with `min_doc_count: 0` and a `missing` value drop the
synthetic bucket when at least one shard contributes zero matching documents.
A single-shard empty index returns `aggregations.by_missing.buckets: []` instead
of the expected `[{ "key": "N/A", "doc_count": 1 }]`.

### Root cause

The shard-local aggregator takes an empty-segment fast path that returns before
building buckets. That fast path is correct for `min_doc_count >= 1` (empty
buckets are discarded anyway) but wrong for `min_doc_count == 0`: the synthetic
(and `missing`) bucket that should exist for every shard is produced by no shard,
so the coordinating-node reduce in `InternalTerms.reduce(...)` has nothing to
merge. Mechanism and `git blame` (introduced in #4567) are in the linked issue.

### Fix

Gate the empty-segment fast path on `min_doc_count != 0` in
`TermsAggregator.doPostCollection()` (one guard condition). Behavior for
`min_doc_count >= 1` is byte-for-byte unchanged.

### Testing

- Unit: `TermsAggregatorTests.testMissingWithMinDocCountZeroOnEmptyShard`
  (red on main, green here) + a negative control with `min_doc_count: 1`.
- REST-YAML: `search.aggregation/terms_missing_min_doc_count.yml`.
- 50 iters + 10 seeds, no flakes. `./gradlew precommit check` green locally.

### Backward compatibility

No serialization change; no `Version` guard needed. The only observable change is
the intended one: the previously-missing bucket is now returned. No new settings.

### Related Issues

Resolves #NNNN

Notice it is scannable: a reviewer reads four headings and knows whether to trust the change before reading a line of code. A strong description routinely halves review rounds.

Fixes / Resolves: what actually auto-closes

Resolves #NNNN and Fixes #NNNN are GitHub closing keywords (the full set: close/closes/closed, fix/fixes/fixed, resolve/resolves/resolved). They read the same, but the auto-close behavior has three sharp edges worth knowing before you rely on them:

  • They close issues, not PRs. Fixes #NNNN where #NNNN is an issue auto-closes that issue when the PR merges. Fixes #NNNN where #NNNN is a PR just creates a cross-link — it does not close the other PR. If you mean "this supersedes PR #NNNN," say so in prose; the keyword does nothing there.
  • They only fire on merge to the default branch. GitHub auto-closes a linked issue only when the closing keyword lands on the repo's default branch (main). This is the one that bites: your backport 2.x PR carrying Resolves #NNNN will not close the issue, because it merges into 2.x, not main. Put the closing keyword on the main PR and use a plain reference (Related to #NNNN, no keyword) on the backport, so the issue isn't double-counted or confusingly re-closed.
  • One keyword closes one issue. Fixes #A and #B only closes #A. To close both, repeat the keyword: Fixes #A, fixes #B.

So the rule is: exactly one closing keyword, on the main PR, pointing at the issue you are resolving. Everything else is a plain reference.

Open the PR, Labels, and Reviewers

git push -u fork fix/NNNN-terms-missing-min-doc-count

gh pr create --repo opensearch-project/OpenSearch \
  --base main \
  --title "Fix terms agg dropping missing bucket with min_doc_count=0 on empty shards" \
  --body-file capstone-work/pr-body.md \
  --label "bug"

# if it should also ship in the maintenance line, add the backport label:
gh pr edit --repo opensearch-project/OpenSearch <PR-number> \
  --add-label "backport 2.x"

Then, on the PR:

  • Labels and the backport mechanic. Apply bug (and a component label if the repo uses them, e.g. Search:Aggregations). If the fix should ship in the maintenance line too, add the backport 2.x label — the label name is literally backport <branch>, matching the target branch. The mechanic is automated by the backport.yml GitHub workflow, and the sequence matters:

    1. Add the backport 2.x label while the PR is open, so the backport workflow runs as one of the PR's checks before merge.
    2. When the PR merges to main, the workflow opens a backport PR to 2.x automatically by cherry-picking your commits.
    3. That backport PR runs its own CI and needs its own review/merge — the bot opens it, it does not merge it.

    Only hand-cherry-pick if the bot fails (e.g. the cherry-pick conflicts); then you fix the conflict on a backport/2.x/... branch yourself. Some labels you cannot self-apply — ask in the PR or on Slack and a maintainer will add them.

  • Reviewers. Request the maintainers who own the area. Find them in MAINTAINERS.md and via git log/git blame on the files you touched (the author of the introducing PR #4567 is a natural reviewer — @-mention them).

  • Link the issue. Resolves #NNNN in the body does the auto-close; also make sure the issue is referenced so the cross-links resolve both ways.

Keep CI Green

After you push, gradle-check runs (triggered by automation/maintainers). Your job:

  • It should be green on the first run because you ran ./gradlew check locally in Step 7.
  • If it's red, follow the Step 7 "Reading CI" procedure: open the logs, grab the REPRODUCE WITH: line, reproduce locally, fix, push. Don't push speculative fixes.
  • The DCO and CHANGELOG checks are independent of gradle-check — a red DCO/ CHANGELOG check is a mechanics problem (missing sign-off, missing entry), not a test problem. Fix the mechanics.
  • Re-validate after every push. "I fixed the test" is a claim until CI agrees.

Deliverable for Step 8

  • A fork + a descriptively-named branch off current origin/main.
  • All commits DCO-signed (git commit -s); Signed-off-by: matches your git identity; DCO check green.
  • A one-line CHANGELOG.md entry under the correct heading.
  • The PR template filled completely and honestly.
  • A scannable PR description: problem → root cause → fix → tests → BWC → Resolves #NNNN.
  • Labels applied (bug, backport 2.x if relevant), reviewers requested, issue linked.
  • gradle-check, DCO, and CHANGELOG checks green.

Validation / Self-check

Before advancing to Step 9:

  1. Run git log --show-signature-style review: every commit has a Signed-off-by: trailer matching your identity, and the DCO check is green.
  2. CHANGELOG.md has exactly one new line, under the right heading, user-facing, linking the PR.
  3. The PR template has no unanswered prompt or unexplained empty checkbox.
  4. A stranger could read only the PR description and correctly state the bug, the cause, the fix, and the BWC impact.
  5. You requested the right reviewers (area maintainers + the introducing-PR author) and applied the right labels.
  6. Resolves #NNNN is present so the issue auto-closes on merge.
  7. CI (gradle-check) is green, or you have an in-progress, locally-reproduced fix for any red — not a speculative push.

Then go to Step 9: GitHub Documentation.