diff --git a/.claude/skills/create-java-pr/SKILL.md b/.claude/skills/create-java-pr/SKILL.md index e2a9b9bc785..17b5839d88a 100644 --- a/.claude/skills/create-java-pr/SKILL.md +++ b/.claude/skills/create-java-pr/SKILL.md @@ -7,7 +7,8 @@ description: Create a pull request in sentry-java. Use when asked to "create pr" Prepare local changes and create a pull request for the sentry-java repo. -**Required reading:** Before proceeding, read `.cursor/rules/pr.mdc` for the full PR and stacked PR workflow details. That file is the source of truth for PR conventions, stack comment format, branch naming, and merge strategy. +**For stacked PRs:** read `references/stacked-prs.md` before proceeding. It is the source of truth for +stack structure, title naming, stack list format, and merge strategy. ## Step 0: Determine PR Type From Git Branch Context @@ -66,7 +67,7 @@ git checkout -b / Derive the branch name from the changes being made. Use `feat/`, `fix/`, `ref/`, etc. matching the commit type conventions. -**For stacked PRs:** For the first PR in a new stack, first create and push the collection branch (see `.cursor/rules/pr.mdc` § "Creating the Collection Branch"), then branch the PR off it. For subsequent PRs, branch off the previous stack branch. Use the naming conventions from `.cursor/rules/pr.mdc` § "Branch Naming". +**For stacked PRs:** For the first PR in a new stack, first create and push the collection branch (see `references/stacked-prs.md` § "Why a Collection Branch"), then branch the PR off it. For subsequent PRs, branch off the previous stack branch. Give every branch in the stack a shared prefix naming the feature, with a descriptive suffix per PR. **CRITICAL: Never merge, fast-forward, or push commits into the collection branch.** It stays at its initial position until the user merges stack PRs through GitHub. Updating it will auto-merge and destroy the entire PR stack. @@ -88,7 +89,13 @@ Check for uncommitted changes: git status --porcelain ``` -If there are uncommitted changes, invoke the `sentry-skills:commit` skill to stage and commit them following Sentry conventions. +If there are uncommitted changes, invoke the `sentry-skills:commit` skill to stage and commit them following [Sentry commit message conventions](https://develop.sentry.dev/engineering-practices/commit-messages/): + +``` +(): +``` + +Allowed types: `feat`, `fix`, `ref`, `chore`, `docs`, `test`, `perf`, `build`, `ci`, `style`, `meta`, `license` **Important:** When staging, ignore changes that are only relevant for local testing and should not be part of the PR. Common examples: @@ -114,39 +121,28 @@ If the push fails due to diverged history, ask the user how to proceed rather th ## Step 5: Create PR -Invoke the `sentry-skills:create-pr` skill to create a draft PR. When providing the PR body, use the repo's PR template structure from `.github/pull_request_template.md`: +Invoke the `sentry-skills:create-pr` skill to create a draft PR. + +Read `.github/pull_request_template.md` and use it as the PR body structure — it is the single source +of truth for the sections and checklist, so never reproduce it from memory. Fill in each section based +on the changes being PR'd, drop the HTML comment hints, and check any checklist items that apply. + +**PR title format** — same as the commit subject (Step 3): ``` -## :scroll: Description - - -## :bulb: Motivation and Context - - -## :green_heart: How did you test it? - - -## :pencil: Checklist -- [ ] I added GH Issue ID _&_ Linear ID -- [ ] I added tests to verify the changes. -- [ ] No new PII added or SDK only sends newly added PII if `sendDefaultPII` is enabled. -- [ ] I updated the docs if needed. -- [ ] I updated the wizard if needed. -- [ ] Review from the native team if needed. -- [ ] No breaking change or entry added to the changelog. -- [ ] No breaking change for hybrid SDKs or communicated to hybrid SDKs. - -## :crystal_ball: Next steps +(): ``` -Fill in each section based on the changes being PR'd. Check any checklist items that apply. +Examples: +- `feat(core): Add structured logging support` +- `fix(android): Prevent crash on API 21 when registering receiver` **For stacked PRs:** - Pass `--base ` so the PR targets the previous branch (first PR in a stack targets the collection branch). -- Use the stacked PR title format: `(): [ ] ` (see `.cursor/rules/pr.mdc` § "PR Title Naming"). -- Include the stack list at the top of the PR body, before the `## :scroll: Description` section (see `.cursor/rules/pr.mdc` § "Stack List in PR Description" for the format). -- Add a merge method reminder at the very end of the PR body (see `.cursor/rules/pr.mdc` § "Stack List in PR Description" for the exact text). This only applies to stack PRs, not the collection branch PR. +- Use the stacked PR title format: `(): [ ] ` (see `references/stacked-prs.md` § "PR Title Naming"). +- Include the stack list at the top of the PR body, before the `## :scroll: Description` section (see `references/stacked-prs.md` § "Stack List in PR Description" for the format). +- Add a merge method reminder at the very end of the PR body (see `references/stacked-prs.md` § "Stack List in PR Description" for the exact text). This only applies to stack PRs, not the collection branch PR. Then continue to Step 5.5 (stacked PRs only) or Step 6. @@ -154,13 +150,9 @@ Then continue to Step 5.5 (stacked PRs only) or Step 6. Skip this step for standalone PRs. -After creating the PR, update the PR description on **every other PR in the stack — including the collection branch PR** — so all PRs have the same up-to-date stack list. Follow the format and commands in `.cursor/rules/pr.mdc` § "Stack List in PR Description". +After creating the PR, update the PR description on **every other PR in the stack — including the collection branch PR** — so all PRs have the same up-to-date stack list. Follow the format and commands in `references/stacked-prs.md` § "Stack List in PR Description". -**Important:** When updating PR bodies, never use shell redirects (`>`, `>>`) or pipes (`|`) or compound commands (`&&`). These create compound shell expressions that won't match permission patterns. Instead: -- Use `gh pr view --json body --jq '.body'` to get the body (output returned directly) -- Use the `Write` tool to save it to a temp file -- Use the `Edit` tool to modify the temp file -- Use `gh pr edit --body-file /tmp/pr-body.md` to update +Edit each body using the procedure in § "Editing PR Descriptions" below. ## Step 6: Update Changelog @@ -190,6 +182,8 @@ Add an entry to `CHANGELOG.md` under the `## Unreleased` section. Create the subsection under `## Unreleased` if it does not already exist. +**When rebasing:** A rebase onto `main` can land your branch after a release was cut, where the `## Unreleased` heading your entry lived under has since been renamed to that version number. If that happens, move your new entry into an `## Unreleased` section at the top of `CHANGELOG.md` (create the section if it no longer exists) so it is not left under an already-released version. + #### Entry format ```markdown @@ -210,8 +204,14 @@ git push ### No changelog needed -If no changelog entry is needed, add `#skip-changelog` to the PR description to disable the changelog CI check: +If no changelog entry is needed, append `#skip-changelog` to the end of the PR description to disable +the changelog CI check, using the procedure in § "Editing PR Descriptions" below. + +## Editing PR Descriptions + +Do not use shell redirects (`>`, `>>`), pipes (`|`), or compound commands (`&&`, `||`). These create +compound shell expressions that won't match permission patterns. Instead: -1. Get the current body: `gh pr view --json body --jq '.body'` -2. Use the `Write` tool to save the output to `/tmp/pr-body.md`, appending `\n#skip-changelog\n` at the end -3. Update: `gh pr edit --body-file /tmp/pr-body.md` +1. Read the body with `gh pr view --json body --jq '.body'` (output is returned directly) +2. Use the `Write` tool to save it to `/tmp/pr-body.md`, and the `Edit` tool to modify it +3. Update with `gh pr edit --body-file /tmp/pr-body.md` diff --git a/.claude/skills/create-java-pr/references/stacked-prs.md b/.claude/skills/create-java-pr/references/stacked-prs.md new file mode 100644 index 00000000000..56221fe4912 --- /dev/null +++ b/.claude/skills/create-java-pr/references/stacked-prs.md @@ -0,0 +1,86 @@ +# Stacked PRs + +Stacked PRs split a large feature into small, easy-to-review PRs where each builds on the previous +one. The general mechanics are the standard [Graphite](https://graphite.dev/) stacking workflow — +this file covers only what is specific to sentry-java. + +## Why a Collection Branch + +``` +main ← collection-branch ← stack-pr-1 ← stack-pr-2 ← stack-pr-3 ← ... +``` + +A **collection branch** is created from `main` and targets `main`. The first stack PR targets it +rather than `main`, and each later PR targets the previous stack PR's branch. + +It exists because PRs targeting `main` are **squash**-merged, which causes repeated merge conflicts +when syncing a stack. Stack PRs are therefore **merge-committed** into the collection branch, and +only the collection branch is squash-merged into `main` at the end — giving `main` one clean commit +for the whole feature. + +Create it with an empty commit, so GitHub allows opening a PR: + +```bash +git commit --allow-empty -m "collection: " +``` + +Push it and open its PR against `main` right away — it is the PR the whole stack is eventually +squash-merged through, and it carries the stack list like every other PR. Give it a plain title +(`(): `, no `[ ]` bracket) and no merge method reminder. + +## Rules That Will Destroy a Stack If Broken + +**Never update the collection branch yourself.** Never merge, fast-forward, or push stack branch +commits into it. It stays at its initial position (the empty commit on `main`) until the user merges +stack PRs through GitHub one by one. Fast-forwarding it makes GitHub auto-merge and delete every +stack PR branch, destroying the entire stack. + +**Never amend or force-push a stack branch.** No `git commit --amend`, `--force`, or +`--force-with-lease` on a branch that is part of a stack — a force-push can cause GitHub to +auto-merge or auto-close the other PRs in the stack. If a commit needs fixing, add a fixup commit. + +**Sync only between adjacent stack branches**, by merging forward — never into the collection branch. +Prefer merge over rebase; only rebase if explicitly requested. + +**Do not merge PRs.** Only the user merges them, bottom to top. + +## PR Title Naming + +Include the topic name and a sequential number in brackets: + +``` +(): [ ] +``` + +Examples: +- `feat(core): [Global Attributes 1] Add scope-level attributes API` +- `feat(core): [Global Attributes 2] Wire scope attributes into LoggerApi and MetricsApi` + +## Stack List in PR Description + +Every PR in the stack — **including the collection branch PR** — must have a stack list **at the top +of its description**, before the `## :scroll: Description` section. When a PR is added, update the +description on **all** PRs in the stack. The stack list is also how you enumerate a stack: read it +off any PR body rather than guessing from branch names, which may use different prefixes. + +```markdown +## PR Stack () + +- #5118 +- #5120 +- #5121 + +--- +``` + +No status column — GitHub already shows that. The `---` separates the stack list from the rest of +the description. + +**Merge method reminder:** on stack PRs (not the collection branch PR), end the description with: + +```markdown +> ⚠️ **Merge this PR using a merge commit** (not squash). Only the collection branch is squash-merged into main. +``` + +Updating every PR's stack list means editing several descriptions — follow the procedure in +`SKILL.md` § "Editing PR Descriptions". diff --git a/.cursor/rules/pr.mdc b/.cursor/rules/pr.mdc deleted file mode 100644 index 3a37ecc15f8..00000000000 --- a/.cursor/rules/pr.mdc +++ /dev/null @@ -1,264 +0,0 @@ ---- -alwaysApply: false -description: Pull request creation, stacked PRs, and PR workflow ---- - -# Pull Request Rules - -## Creating a Pull Request - -### Step 1: Ensure Feature Branch - -If on `main`, create and switch to a new branch: - -```bash -git checkout -b / -``` - -Branch names use `feat/`, `fix/`, `ref/`, etc. matching the commit type. - -### Step 2: Format Code and Regenerate API Files - -```bash -./gradlew spotlessApply apiDump -``` - -This is **required** before every PR. Fix any failures before continuing. - -### Step 3: Commit Changes - -Use `git status --porcelain` to review changes. Ignore files only relevant for local testing (hardcoded debug toggles, sample app config, `.env` files). Restore those with `git checkout -- `. - -Follow [Sentry commit message conventions](https://develop.sentry.dev/engineering-practices/commit-messages/): - -``` -(): -``` - -Allowed types: `feat`, `fix`, `ref`, `chore`, `docs`, `test`, `perf`, `build`, `ci`, `style`, `meta`, `license` - -- Use imperative present tense ("add" not "added") -- Capitalize subject, no trailing period -- Keep under 100 characters - -### Step 4: Push - -```bash -git push -u origin HEAD -``` - -If push fails due to diverged history, ask the user — do not force-push. - -### Step 5: Create PR - -Create a draft PR using the repo's PR template: - -```markdown -## :scroll: Description - - -## :bulb: Motivation and Context - - -## :green_heart: How did you test it? - - -## :pencil: Checklist -- [ ] I added GH Issue ID _&_ Linear ID -- [ ] I added tests to verify the changes. -- [ ] No new PII added or SDK only sends newly added PII if `sendDefaultPII` is enabled. -- [ ] I updated the docs if needed. -- [ ] I updated the wizard if needed. -- [ ] Review from the native team if needed. -- [ ] No breaking change or entry added to the changelog. -- [ ] No breaking change for hybrid SDKs or communicated to hybrid SDKs. - -## :crystal_ball: Next steps -``` - -### Step 6: Update Changelog - -Add an entry to `CHANGELOG.md` under `## Unreleased` in the appropriate subsection: - -| Change Type | Subsection | -|---|---| -| New feature | `### Features` | -| Bug fix | `### Fixes` | -| Refactoring, internal cleanup | `### Internal` | -| Dependency update | `### Dependencies` | - -Entry format: - -```markdown -- ([#](https://github.com/getsentry/sentry-java/pull/)) -``` - -**When rebasing:** A rebase onto `main` can land your branch after a release was cut, where the `## Unreleased` heading your entry lived under has since been renamed to that version number. If that happens, move your new entry into an `## Unreleased` section at the top of `CHANGELOG.md` (create the section if it no longer exists) so it is not left under an already-released version. - -Commit changelog separately: - -```bash -git add CHANGELOG.md && git commit -m "changelog" && git push -``` - -### PR Title Format - -Follow the commit message format: - -``` -(): -``` - -Examples: -- `feat(core): Add structured logging support` -- `fix(android): Prevent crash on API 21 when registering receiver` - ---- - -## Stacked PRs - -Stacked PRs split a large feature into small, easy-to-review PRs where each builds on the previous one. This follows the same concept as the [Graphite](https://graphite.dev/) stacking workflow. - -### Structure - -``` -main ← collection-branch ← stack-pr-1 ← stack-pr-2 ← stack-pr-3 ← ... -``` - -- A **collection branch** is created from `main` and targets `main`. It serves as the base for the entire stack. -- The first PR in the stack targets the collection branch (not `main`). -- Each subsequent PR targets the previous stack PR's branch as its base. -- Each PR contains only incremental changes on top of the previous one. - -The collection branch exists so that individual stack PRs can be **merge-committed** (not squashed). PRs targeting `main` use squash merging, but that causes repeated merge conflicts when syncing the stack. Merge commits on non-`main` branches avoid this. The collection branch itself is squash-merged into `main` at the end. - -### Branch Naming - -Prefer a shared prefix for the feature, with descriptive suffixes per PR. The collection branch uses the shared prefix. The type prefix (`feat/`, `fix/`, etc.) may vary depending on the nature of each PR's changes: - -``` -feat/scope-attributes # collection branch → targets main -feat/scope-attributes-api # PR 1 → targets collection branch -feat/scope-attributes-logger # PR 2 → targets PR 1 -fix/attribute-type-detection # PR 3 (fix, different name — that's fine) → targets PR 2 -``` - -### PR Title Naming - -Include the topic name and a sequential number in brackets: - -``` -(): [ ] -``` - -Examples: -- `feat(core): [Global Attributes 1] Add scope-level attributes API` -- `feat(core): [Global Attributes 2] Wire scope attributes into LoggerApi and MetricsApi` -- `feat(samples): [Global Attributes 3] Showcase scope attributes in Spring Boot 4 sample` - -### Finding All PRs in a Stack - -Do **not** rely on branch name patterns — later PRs in a stack may use different prefixes or naming. Instead: - -1. Find the PR for the current branch: - ```bash - gh pr list --head "$(git branch --show-current)" --json number,title,baseRefName --jq '.[0]' - ``` -2. Read the PR description — the stack list is at the top of the body. -3. If there is no stack list yet, walk the chain in both directions: - ```bash - # Find the PR whose head branch is the current PR's base (go up) - gh pr list --head --json number,title,baseRefName - - # Find PRs whose base branch is the current PR's head (go down) - gh pr list --base --json number,title,headRefName - ``` - Repeat until you reach the collection branch going up and find no more PRs going down. - -### Creating the Collection Branch - -Before the first stacked PR, create the collection branch with an empty commit (so GitHub allows opening a PR) and create the collection PR: - -```bash -git checkout main -git checkout -b feat/ -git commit --allow-empty -m "collection: " -git push -u origin HEAD -gh pr create --base main --draft --title "(): " --body "Collection PR for the stack. Squash-merge this once all stack PRs are merged." -``` - -**CRITICAL: Do NOT manually update the collection branch.** Never merge, fast-forward, or push stack branch commits into the collection branch. The collection branch stays at its initial position (the empty commit on `main`) until the user merges individual stack PRs into it one by one through GitHub. If you fast-forward the collection branch to include stack commits, GitHub will auto-merge and delete all stack PR branches, destroying the entire stack. - -### Creating a New Stacked PR - -1. Start from the tip of the previous stack branch (or the collection branch for the first PR). -2. Create a new branch, make changes, format, commit, and push. -3. Create the PR with `--base ` (the collection branch for the first PR): - ```bash - gh pr create --base feat/previous-branch --draft --title "(): [ ] " --body "..." - ``` -4. Add the stack list to the top of the new PR's description and update it on all existing PRs in the stack (see below). - -### Stack List in PR Description - -Every PR in the stack — **including the collection branch PR** — must have a stack list **at the top of its description** (before the `## :scroll: Description` section). When a new PR is added, update the description on **all** PRs in the stack and on the collection branch PR. - -Format: - -```markdown -## PR Stack () - -- #5118 -- #5120 -- #5121 - ---- -``` - -No status column — GitHub already shows that. The `---` separates the stack list from the rest of the PR description. - -**Merge method reminder:** On stack PRs (not the collection branch PR), add the following line at the very end of the PR description: - -```markdown -> ⚠️ **Merge this PR using a merge commit** (not squash). Only the collection branch is squash-merged into main. -``` - -This does not apply to standalone PRs or the collection branch PR. - -To update the PR description, use `--body-file` to avoid shell quoting issues with special characters in the body. - -**Important:** Do not use shell redirects (`>`, `>>`, `|`) or compound commands (`&&`, `||`). These create compound shell expressions that won't match permission patterns. Instead, use the `Write` and `Edit` tools for file manipulation: - -1. Read the current body with `gh pr view --json body --jq '.body'` (the output is returned directly — use the `Write` tool to save it to `/tmp/pr-body.md`) -2. Use the `Edit` tool to prepend or replace the stack list section in `/tmp/pr-body.md` -3. Update the description: `gh pr edit --body-file /tmp/pr-body.md` - -### Merging Stacked PRs (done by the user, not the agent) - -Individual stack PRs are merged in order from bottom to top (PR 1 first, then PR 2, etc.) using **merge commits** (not squash). After each merge, the next PR's base automatically becomes the merged branch's target. GitHub handles rebasing onto the new base. - -Once all stack PRs are merged into the collection branch, the collection PR is **squash-merged** into `main`. This gives `main` a clean single commit for the entire feature. - -**Do not merge PRs.** Only the user merges PRs. - -### Syncing the Stack - -When a base PR changes (e.g. after addressing review feedback on PR 1), merge the changes forward through the stack **between adjacent stack PR branches only**: - -```bash -# On the branch for PR 2 -git checkout feat/scope-attributes-logger -git merge feat/scope-attributes-api -git push - -# On the branch for PR 3 -git checkout feat/scope-attributes-sample -git merge feat/scope-attributes-logger -git push -``` - -**Never merge into the collection branch.** Syncing only happens between stack PR branches. The collection branch is untouched until the user merges PRs through GitHub. - -Prefer merge over rebase — it preserves commit history, doesn't invalidate existing review comments, and avoids the need for force-pushing. Only rebase if explicitly requested. - -**Never amend or force-push stack branches.** Do not use `git commit --amend`, `--force`, or `--force-with-lease` on branches that are part of a stack. Amending a pushed commit requires a force-push, which can cause GitHub to auto-merge or auto-close other PRs in the stack. If a commit needs fixing, add a new fixup commit instead. diff --git a/AGENTS.md b/AGENTS.md index fd1fced0df6..42bc6677d17 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -20,7 +20,6 @@ rule file in `.cursor/rules/`: | `continuous_profiling_jvm` | `sentry-async-profiler`, `IContinuousProfiler`, `ProfileChunk`, JFR files, `ProfileLifecycle` | | `opentelemetry` | `sentry-opentelemetry-*`, agent vs agentless, span processing, sampling, context propagation | | `new_module` | Adding a new integration or sample module | -| `pr` | Creating pull requests, stacked PRs, changelog entries | | `e2e_tests` | System tests, sample applications, `system-test-runner.py`, mock Sentry server | Rules can be combined — a tracing scope issue may need both `scopes` and `opentelemetry`. @@ -99,7 +98,7 @@ make systemTest 4. **High-level communication**: Give high-level explanations of changes made, not step-by-step descriptions 5. **Simplicity first**: Make every task and code change as simple as possible. Avoid massive or complex changes. Impact as little code as possible. 6. **Format and regenerate**: Once done, format code and regenerate .api files: `./gradlew spotlessApply apiDump` -7. **Propose commit**: As final step, git stage relevant files and propose (but not execute) a single git commit command +7. **Propose commit**: As final step, git stage relevant files and propose (but not execute) a single git commit command. This applies to implementation work; when the task is to open a PR, the `create-java-pr` skill takes over from here and does commit, push, and open it. ## Repository Skills @@ -225,7 +224,9 @@ gh pr view --json url -q '.url' ### Changelog -User-facing changes get an entry under the `## Unreleased` section of `CHANGELOG.md`. When rebasing onto `main`, a release may have renamed the `## Unreleased` heading your entry was under to a version number — if so, move your entry back into an `## Unreleased` section at the top of the file (create it if it no longer exists). See `.cursor/rules/pr.mdc` for the full changelog and PR workflow. +User-facing changes get an entry under the `## Unreleased` section of `CHANGELOG.md`. The +`create-java-pr` skill is the source of truth for the full changelog and PR workflow, including +subsection selection and the rebase caveat when a release renames `## Unreleased`. ## Useful Resources