Step 9: GitHub Documentation

A PR is not a fire-and-forget artifact. Between "opened" and "merged" sits the review cycle, and how you run that is most of what a maintainer remembers about you. This step covers the documentation and conversation hygiene that surrounds the code: keeping the issue and PR honest as the change evolves, getting the CHANGELOG heading right, handling the separate documentation-website repo, responding to review rounds cleanly, and the merge + backport-bot endgame.

The rule that governs this step: the PR conversation and the issue should, at any moment, accurately describe the current state of the change — never a stale earlier version of it.


Update the Issue

The issue is where triagers, future searchers, and the maintainer who assigns the review look first. Keep it current:

  • Comment when you start. "Working on this — have a reproducer and a candidate fix, PR incoming." This claims the work informally (OpenSearch doesn't hard-assign most issues, but a comment prevents duplicate effort) and signals you're active.
  • Link the PR the moment it's open (the PR's Resolves #NNNN cross-links automatically, but a one-line comment helps).
  • Drop the untriaged confusion. If the issue carried untriaged, your reproduction and root-cause comment is exactly what a maintainer needs to triage it — paste the symptom, trigger conditions, and affected versions you nailed down in Steps 2 and 4.
  • Don't close it yourself. Resolves #NNNN in the merged PR auto-closes it. Closing it manually before merge just confuses the cross-link.

PR Conversation Hygiene

The PR description is a living document, not a first-draft you abandon. After any material change to the approach (you switched fix directions, you narrowed the scope), edit the top-of-PR description to match. A reviewer who returns after a week should not have to reconstruct the current design from 30 comments.

  • Keep the description's "Fix" and "Testing" sections accurate to the latest diff.
  • When you push changes in response to feedback, leave a short comment summarizing what changed ("Addressed @reviewer: moved the guard into doPostCollection, added the negative-control test").
  • Resolve review threads only when you've actually pushed the corresponding change — and resolve them yourself with a substantive reply, not silently.

CHANGELOG: Changed vs Fixed

You added a CHANGELOG line in Step 8. Make sure it's under the right heading, because reviewers and release managers read these for the release notes, and the distinction matters:

HeadingUse forExample
FixedA bug: behavior that was wrong by the documented/intended contract"Fix terms agg dropping the missing bucket with min_doc_count: 0"
ChangedA deliberate behavior change that wasn't strictly a bug, or a change users may notice and must be told about"Change default search.max_buckets enforcement to apply during reduce"
AddedNew capability / setting / API"Add search.concurrent_segment_search.enabled index setting"
Deprecated / Removed / SecurityAs named

The aggregation fix is unambiguously Fixed. But ask yourself the honest question: does anyone depend on the current (buggy) behavior? If realistically yes — if scripts parse the empty-buckets response — then even a bug fix has a Changed-flavored blast radius, and you should flag that in the PR so a maintainer can decide whether it needs a migration note. When in doubt, raise it in the PR conversation; don't decide unilaterally.


Documentation-Website Implications

OpenSearch user docs do not live in the engine repo. They live in a separate repo, opensearch-project/documentation-website (the source of opensearch.org/docs). The engine PR and the docs PR are independent.

Decide whether your change needs a docs update:

Your change…Docs action
Fixes a bug so the API now matches its documented behaviorUsually no docs change — the docs already describe the correct behavior; you just made the code match. Note "no docs change needed; behavior now matches existing docs" in the PR.
Changes documented behavior, a default, or an error message users seeOpen a docs PR in documentation-website, and link it from the engine PR ("Docs: opensearch-project/documentation-website#MMM").
Adds a setting, API, or parameterOpen a docs PR documenting it. New surface with no docs is incomplete.

For the aggregation fix, the docs already state that min_doc_count: 0 produces empty buckets; the code was wrong, not the docs. So: no docs PR, but say so explicitly. If you do open a docs PR, it follows the same DCO + template flow as the engine PR, in the docs repo.


Responding to Review Rounds

Reviews come in rounds. The mechanics of responding cleanly — without losing review context or making reviewers re-read the whole diff — matter as much as the content of your replies. (The content — tone, when to push back, how to handle disagreement — is the subject of the responding-to-feedback chapter; read it. This section is the GitHub-mechanics half.)

Fixup commits during review, squash at the end

During active review, push separate fixup commits rather than amending and force-pushing — this preserves the per-comment diff so a reviewer can see exactly what changed since their last look. GitHub shows "changes since you last reviewed," which only works if you didn't rewrite history.

# Make the change a reviewer asked for, as its own signed commit.
git commit -s -m "Address review: extract guard into helper for readability"
git push fork fix/NNNN-terms-missing-min-doc-count

When the PR is approved and ready to merge, the maintainer typically squash- merges (OpenSearch squashes PRs into a single commit), so your fixup commits collapse automatically — you usually don't need to squash by hand. If the project asks you to clean history before merge:

git rebase -i origin/main      # mark fixups as 'fixup'/'squash'
git push --force-with-lease fork fix/NNNN-terms-missing-min-doc-count

Warning: Use --force-with-lease, never bare --force — it refuses to overwrite if someone (or the backport bot) pushed to your branch since your last fetch. And after any force-push, comment to tell reviewers you rewrote history and summarize what changed, or you'll cost them a full re-review.

Each comment gets a resolution

For every review comment, do one of two things, visibly:

  1. Push a change that addresses it, then reply "done in <short-sha>" and resolve the thread.
  2. Make the technical case for why the current code is correct — calmly, with reasoning, not "I think mine is fine." If the reviewer agrees, they resolve; if not, you've at least surfaced a real design discussion.

Don't let stylistic comments slide unacknowledged — even a "good catch, fixed" keeps the cadence healthy.


Keep CI Green Through Review

Every push re-runs gradle-check. A change that addresses one reviewer's comment can break a test in a corner you didn't touch. Re-validate locally (Step 7) after every review-driven change before you push. A reviewer who approves a green PR, only to see it go red on your next push, loses confidence fast.


Merge and Backport

When you have the required approval(s) and green CI, a maintainer merges (squash). Then:

  • The issue auto-closes via Resolves #NNNN. Confirm it did.
  • The backport bot runs if you applied a backport <branch> label (e.g. backport 2.x). It opens a backport PR against the maintenance branch. Watch it:
    • If it applies cleanly, it opens a backport PR automatically — review it, make sure its CI is green, and it merges with maintainer approval.
    • If it conflicts, the bot comments that it couldn't cherry-pick. Now you backport by hand: git checkout 2.x && git cherry-pick -x <merge-sha>, resolve conflicts, push a branch, open the backport PR yourself, and link it.
  • Add the merged commit and backport PRs to the issue/PR cross-links so the whole story is navigable later.

Then say thanks — to the reviewer, in the PR. It costs nothing and it is how communities stay places people want to contribute to. (More on the social layer in community interaction and the release process, which explains which release your fix actually ships in.)


Deliverable for Step 9

  • The issue is updated (start comment, repro/root-cause pasted, PR linked) and left to auto-close on merge.
  • The PR description is current to the latest diff; material changes are summarized in comments.
  • The CHANGELOG entry is under the correct heading (Fixed vs Changed), with the blast-radius question explicitly considered.
  • Docs implication decided: a documentation-website PR opened and linked, or an explicit "no docs change needed" note.
  • Review rounds handled with fixup commits (not silent force-pushes), each thread resolved with a substantive reply, CI re-validated after every push.
  • Post-merge: issue auto-closed, backport PR (bot or manual) green and linked.

Validation / Self-check

Before advancing to Step 10:

  1. At any moment during review, the PR description accurately described the current change, not an earlier draft.
  2. Your CHANGELOG entry is under the heading you can defend, and you considered whether anyone depends on the old behavior.
  3. You made the correct docs call (PR in documentation-website, or a documented "no docs needed") — not silence.
  4. You pushed fixup commits during review and only force-pushed (with --force-with-lease) when asked, always with a summarizing comment.
  5. Every review thread is resolved with a change or a reasoned reply — none left dangling.
  6. CI was green after every push, not just the first.
  7. Post-merge, the issue closed, and any backport label produced a green, linked backport PR (bot or hand-cherry-picked on conflict).

Then go to Step 10: Engineering Write-Up.