PR Quality and Preparation

A maintainer reviewing your pull request is making a risk decision: will merging this make the codebase better without introducing a regression, a compatibility break, or a maintenance burden? Everything you do before you click "Create pull request" either lowers that risk or raises it. A high-quality PR is not the one with the cleverest code — it is the one that is easy and safe to say yes to.

This chapter is the checklist a maintainer-ready OpenSearch PR meets. It maps directly to Level 2 Lab 2: Prepare a PR and to the project's own CONTRIBUTING.md, DEVELOPER_GUIDE.md, and .github/pull_request_template.md.

What a Maintainer-Ready PR Looks Like

Seven properties, in rough priority order:

1. Focused scope — one logical change. Not "fix bug + reformat 12 files + rename a class."
2. Tests — unit tests always; integration (*IT.java) and/or REST-YAML where behavior crosses nodes or the HTTP boundary.
3. A CHANGELOG.md entry under ## [Unreleased], in the right category.
4. DCO sign-off on every commit (git commit -s).
5. Green local gate — ./gradlew spotlessApply then ./gradlew precommit pass before you push; CI (gradle-check) green after.
6. A clear description tied to an issue — Closes #NNNN, what changed, why, and how it was tested.
7. No collateral damage — no unrelated reformatting, no incidental dependency bumps, no stray debug logging.

The rest of this chapter is each of these in detail.

Focused Scope: One Logical Change

The cardinal rule. A reviewer can hold one change in their head; they cannot hold five. A PR that fixes a bug and reformats a package and renames a field is three reviews wearing one hat, and it will sit unmerged because reviewing it is expensive and risky.

Litmus test before you open the PR:

cd ~/os-src
git diff --stat main...HEAD

If the stat shows files you cannot explain as part of the one thing, split them out. If Spotless reformatted unrelated lines, restrict formatting to your change. If you found a second bug while fixing the first, file a separate issue and a separate PR. "Drive-by" cleanups feel generous; to a reviewer they are noise that hides the real change and expands the blast radius.

Tests: The Non-Negotiable

A PR that changes behavior without a test that would have failed before your change is not ready, full stop. OpenSearch is heavily tested and randomized; maintainers will not merge an untested behavioral change because they cannot trust that a future refactor won't silently break it.

Choose the test tier by what you touched (see Reading the Codebase for the catalog, and the test framework under test/framework/):

Write the failing test first, watch it fail, then make it pass. A test that passes on both the old and new code proves nothing. If you mute a flaky test, never use @Ignore — use @AwaitsFix(bugUrl="https://github.com/opensearch-project/OpenSearch/issues/NNNN") with a real tracking issue.

The CHANGELOG.md Entry

Every user-visible PR adds one line to CHANGELOG.md under the ## [Unreleased] heading, in the correct category. Forgetting this is the single most common reason a CI "changelog verifier" check goes red and a maintainer leaves a one-word review.

sed -n '1,40p' ~/os-src/CHANGELOG.md

The categories follow Keep-a-Changelog:

## [Unreleased 3.x]
### Added
- Add `wait_for_active_shards` support to the foo API ([#5678](https://github.com/opensearch-project/OpenSearch/pull/5678))
### Changed
### Deprecated
### Removed
### Fixed
- Fix incorrect doc_count after delete-by-query in terms agg ([#5679](https://github.com/opensearch-project/OpenSearch/pull/5679))
### Security

Rules: put it under the right ### ; phrase it for a user reading release notes, not for a reviewer; link your PR number; pick the right [Unreleased 3.x] / [Unreleased 2.x] section matching where it lands. Purely internal changes (test-only, build-only) may not need an entry — check the verifier's rules and recent merged PRs.

DCO Sign-Off (Not a CLA)

OpenSearch uses the Developer Certificate of Origin, not a CLA. Every commit must carry a Signed-off-by: line whose name and email match your Git identity. The DCO bot fails the PR if any commit is missing it.

git config user.name  "Your Real Name"
git config user.email "you@example.com"
git commit -s -m "Fix incorrect doc_count after delete-by-query in terms agg"

git commit -s appends:

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

Forgot it on commits you already made? Fix the whole branch in one shot:

git rebase --signoff main      # add Signed-off-by to every commit since main
git push --force-with-lease     # force-pushing your PR branch is normal here

Note: There is no CLA to sign and no JIRA account to create. Your GitHub identity plus a correct DCO sign-off is the entire contributor-identity requirement.

The Local Gate: spotlessApply, precommit, gradle-check

Run the gate locally before pushing. Discovering a checkstyle or license-header failure in CI after a 30-minute build, when you could have caught it in 30 seconds, wastes a review cycle.

cd ~/os-src
# 1. Auto-format (Spotless). Always run this first.
./gradlew spotlessApply

# 2. The fast static gate: checkstyle, forbidden-APIs, license/SPDX headers,
#    dependency checks, loggerUsageCheck, etc.
./gradlew precommit

# 3. Verify formatting is clean (what CI checks):
./gradlew spotlessJavaCheck

# 4. The targeted tests for what you changed:
./gradlew :server:test --tests "org.opensearch.your.ChangedTests"

./gradlew check runs the full gate (all tests + precommit + integ) and is long-running — CI runs the equivalent gradle-check. You do not need to run check end-to-end locally for every push, but you must run precommit and the tests touching your change. New source files need the SPDX Apache-2.0 header:

/*
 * SPDX-License-Identifier: Apache-2.0
 *
 * The OpenSearch Contributors require contributions made to
 * this file be licensed under the Apache-2.0 license or a
 * compatible open source license.
 */

precommit's license check will fail without it.

The PR Description and Template

The repo's .github/pull_request_template.md pre-populates the description. Fill every section; do not delete the template. A reviewer reads the description first and decides whether to invest in the diff.

cat ~/os-src/.github/pull_request_template.md

A strong description has:

• Description: what the change does and why, in plain prose. Link the design discussion.
• Related Issues: Closes #NNNN (this auto-closes the issue on merge) or Related to #NNNN.
• Check List: tick the template's boxes truthfully — tests added, CHANGELOG updated, commits signed off, public docs considered, etc.
• Testing: how you verified it (which ./gradlew tasks, which manual curl).

Tie it to an issue. A PR with no linked issue, for anything beyond a trivial typo, invites the question "was this discussed?" — and if the answer is no, you may be asked to open an issue first.

Backport Labels

OpenSearch maintains release branches (2.x, etc.) alongside main. When a fix should also ship on a maintenance line, the PR gets a backport <branch> label (e.g. backport 2.x), which triggers a backport bot to open the backport PR automatically after merge.

You usually do not self-apply the label — a maintainer decides whether a backport is warranted — but you should propose it in your description ("This is a bug fix and should be backported to 2.x") and be ready to resolve conflicts if the bot's automated backport doesn't apply cleanly. See Responding to Maintainer Feedback for the post-merge backport workflow.

PR Readiness Checklist

Run this before you open the PR. Every box unchecked is a reason it will sit.

Common Reasons PRs Sit Unmerged

Learn these so your PR avoids them:

Validation: Prove You Understand This

1. Take a branch you have and run git diff --stat main...HEAD. Justify every changed file as part of one logical change, or split it.
2. Add a CHANGELOG.md entry for a hypothetical fix in the correct ### category, with the PR-number link.
3. Show the exact commands to (a) sign off every commit on an existing branch and (b) safely force-push the result.
4. Run ./gradlew spotlessApply precommit on a real change and report a clean gate, or fix what it flags.
5. From the PR template, write a complete description for a real or hypothetical change, including Closes #NNNN, the testing section, and a backport recommendation.
6. Name the test tier you would add for each of: a serialization change, a REST contract change, and a cross-node behavior change — with the ./gradlew task for each.

When every box on the readiness checklist is green on the first push, you have made it easy to say yes. The next chapter — Responding to Maintainer Feedback — is what happens after the maintainer reads it.