GitHub Issues and PR Review

This is the chapter you came for if you have ever watched a pull request sit green for a week and wondered what the maintainer was waiting on. OpenSearch review happens entirely on GitHub — no JIRA patches, no mailing-list code review. This chapter is the mechanics of that process from the maintainer's side: the PR lifecycle, the CI gate, who can approve and merge, the MAINTAINERS.md/CODEOWNERS machinery, how the backport bot works, and — most usefully — how a maintainer actually reads your diff.

If PR Quality told you how to write the PR, this tells you how it's received.

The PR Lifecycle

stateDiagram-v2
    [*] --> Opened
    Opened --> CI_running: checks triggered
    CI_running --> CI_failed: a check is red
    CI_failed --> CI_running: push a fix
    CI_running --> In_review: all checks green
    In_review --> Changes_requested: maintainer review
    Changes_requested --> CI_running: address feedback, push
    In_review --> Approved: required approvals met
    Approved --> Merged: maintainer squash-merges
    Merged --> Backport: backport label present
    Backport --> Backport_PR: bot opens PR on 2.x / 1.x
    Backport_PR --> [*]
    Merged --> [*]

From open to merge, every PR passes three gates, in this rough order: DCO + CI green → maintainer review → required approvals → merge. A PR that is missing any one of these does not merge, no matter how good the code is.

Required CI Checks

The core repo gates merges on a set of automated checks. These run on every push to the PR branch. The exact set evolves, but the load-bearing ones are:

Run them yourself before you push, in this order, because they get progressively more expensive:

cd ~/OpenSearch
./gradlew spotlessApply          # auto-fix formatting first
./gradlew precommit              # cheap gate: style, headers, forbidden APIs
./gradlew assemble               # build
./gradlew :server:test --tests "org.opensearch.cluster.ClusterStateTests"  # scoped tests

Note: gradle-check is the big one and it is randomized — it runs tests with a random seed. A green local run does not guarantee a green CI run, and a red CI run may be a flaky test, not your change. If a failure looks unrelated, find the printed -Dtests.seed=... line, reproduce it, and check whether the same test is already labeled flaky-test. Re-running CI is legitimate when you've confirmed the failure is pre-existing; silently re-running until green to mask a real regression is not.

A maintainer will not even start a serious review until CI is green. Red CI is a self-service problem; fix it before you ask for eyes.

Who Reviews, Who Approves, Who Merges

Three documents define authority in any OpenSearch repo:

The model is review-then-merge: a change is reviewed before it lands, not after. Contrast this with commit-then-review (push first, review later) — OpenSearch does not do that on the core engine, because the cost of a bad change in a distributed storage system is too high. A PR needs:

• Green required CI (above), and
• Approval from a maintainer (the required count depends on the repo; the spirit is "an area owner has signed off"). CODEOWNERS ensures the right maintainer is asked.

Only maintainers (those in MAINTAINERS.md) can merge. A non-maintainer's "approve" review is valuable signal and helps the maintainer, but it does not unlock the merge button.

Inspect the authority for an area yourself:

cd ~/OpenSearch
sed -n '1,60p' MAINTAINERS.md          # who the maintainers are
cat .github/CODEOWNERS 2>/dev/null     # path -> owner mapping (if present)
git shortlog -sne --since="12 months ago" -- server/src/main/java/org/opensearch/cluster | head
# ^ who has actually been changing this area lately = de facto reviewer

That last command is the trick: the people who recently and repeatedly touched a directory are the ones who will review changes to it, whether or not CODEOWNERS lists them.

Squash-merge norms

OpenSearch repos squash-merge by default: your PR becomes one commit on main, with the PR title as the commit subject. Practical consequences:

• Write the PR title as the commit message you want in history — imperative, scoped, referencing the area (Fix DiskThresholdDecider off-by-one on relocating shards (#NNNN)).
• Don't obsess over a clean per-commit history inside the PR; it collapses on merge.
• Keep one PR to one logical change. A squashed commit that does three things is a worse history entry and a worse git bisect target later.

How a Maintainer Reads a Diff

When a maintainer opens your PR, they are not reading top to bottom. They are scanning for risk in a fixed priority order. Internalize this order and you write PRs that pass on the first read.

The order is deliberate. Scope and tests come before correctness. A correct change with no test, or a correct change bundled with two unrelated ones, gets sent back before the maintainer evaluates whether the logic is right — because they can't review what they can't isolate, and they won't merge what they can't protect against future regression.

The compatibility lens (row 3) is where "small" changes die. A one-line change to what a Writeable reads off the wire, or to a field in a REST response, can break every node in a cluster mid-upgrade. This is so central it has its own chapter — How Maintainers Think About Compatibility — and a deep dive, serialization-BWC.

Labels and Triage

Issues and PRs move through a label-driven state machine. The two transitions you'll see most:

• untriaged → triaged. A new issue starts untriaged. A maintainer reviews it, assigns a real category (bug, enhancement, RFC), possibly a version label, and removes untriaged. A clean reproduction speeds this up enormously.
• PR labels signal release targeting (v3.1.0) and backporting (backport 2.x).

# Find issues ready for a new contributor:
# github.com/opensearch-project/OpenSearch/issues?q=is:open+label:"good first issue"
# Find still-untriaged issues a maintainer hasn't looked at:
# github.com/opensearch-project/OpenSearch/issues?q=is:open+label:untriaged

The backport Label and the Bot

OpenSearch develops on main (the 3.x line) and maintains older lines on branches: 2.x (maintenance) and 1.x (legacy). A fix that belongs in a released line must be backported to that branch. This is automated:

1. You (or a maintainer) add the backport 2.x label to the PR before or at merge.
2. After the PR squash-merges to main, the backport bot cherry-picks the squashed commit onto a new branch off 2.x and opens a backport PR.
3. That backport PR runs CI and needs its own approval/merge — backporting is not a free pass around review.
4. If the cherry-pick conflicts, the bot fails and comments. You then create the backport manually:

cd ~/OpenSearch
git fetch origin
git checkout -b backport/2.x/my-fix origin/2.x
git cherry-pick -x <squashed-commit-sha>   # resolve conflicts
# ... fix conflicts, keep the CHANGELOG entry under the right release ...
git push origin backport/2.x/my-fix
# open a PR targeting the 2.x branch

Note: The CHANGELOG entry moves with the backport. On main it sits under the [Unreleased 3.x] section; on the 2.x backport it must sit under the [Unreleased 2.x] section. Getting this wrong is a common backport-PR review comment. See The Release Process for how those sections become release notes.

The labels and bot are why a backport must be intentional: shipping a fix to 2.x is a decision, made by adding a label, recorded on the PR, and re-reviewed on the backport.

What Gets a PR Merged Fast vs. Slow

None of the "fast" column is about being a better programmer. It is about respecting the reviewer's time and the engine's compatibility guarantees. That respect, sustained, is what builds the trust that earns you faster reviews and, eventually, a line in MAINTAINERS.md.

Prove You Understand This

1. Name the four load-bearing CI checks and the local ./gradlew command for each. Which is randomized, and how do you reproduce one of its failures?
2. A non-maintainer approves your PR and CI is green. Can it merge? Why or why not?
3. In what order does a maintainer scan a diff, and why do scope and tests come before correctness?
4. Your PR squash-merges to main. You need the fix in the next 2.x patch release. Walk through the label, the bot, and what you do if the cherry-pick conflicts — including the CHANGELOG.
5. Given a directory server/src/main/java/org/opensearch/index/shard/, what single command tells you who is most likely to review a change there?
6. List three things you can do, none of which improve the code itself, that will get your PR merged faster.