From 413ac55d4f3900c07ca614eab3cf94ca5e4d3ce3 Mon Sep 17 00:00:00 2001 From: Dana Jansens Date: Fri, 18 Sep 2026 18:36:58 +0000 Subject: [PATCH] Add rules for working with jj history and prek (#7808) Add rules to not overwrite git/jj history without asking, since this destroys the reviewer's view of things. And some information on dealing with stacks of commits within a single bookmark/PR. Prek can make fixes for whatever caused a failure, and then pass when you run it again, even though the user didn't change anything, and that is now explained. Assisted-by: Opus 5 --- .agents/skills/jj/SKILL.md | 58 ++++++++++++++++++++++++++++++++++++ .agents/skills/prek/SKILL.md | 15 ++++++++++ AGENTS.md | 8 +++++ 3 files changed, 81 insertions(+) diff --git a/.agents/skills/jj/SKILL.md b/.agents/skills/jj/SKILL.md index ddce5b449ad6..4109558390ce 100644 --- a/.agents/skills/jj/SKILL.md +++ b/.agents/skills/jj/SKILL.md @@ -42,3 +42,61 @@ blocking or waiting for terminal paging. and `jj new`. - **Abandon/discard current changes**: `jj --no-pager abandon` - **Rebase current change onto trunk**: `jj --no-pager rebase -o trunk` + +### Working with a stack of changes + +A change is often built as a stack of commits sent up as a single pull request. +The stack is not necessarily based on `trunk`; it may be based on another change +that is itself still in flight. + +> [!WARNING] **Never rewrite the history of a change that has been submitted as +> a pull request.** Reviewers track a PR by its commits, and squashing, +> reordering, or abandoning them discards review that is already in progress. +> This cannot be undone from their side. +> +> Before rewriting history in any other case, propose the exact command and wait +> for confirmation. This applies to `squash`, `rebase`, `abandon`, and +> `describe` on an existing change. + +#### Finding the base of the stack + +Bookmarks delimit the stack. List the bookmarks that are ancestors of the +working copy, nearest first: + +```bash +jj --no-pager log -r '::@ & bookmarks()' +``` + +Reading the result takes care, because two situations produce similar output: + +- **Editing an existing change.** The nearest bookmark names the change being + worked on, and the bookmark below it is the base. +- **Starting a new change.** The commits above the nearest bookmark have no + bookmark of their own yet, so the nearest bookmark is itself the base. + +`trunk` is only ever a base. Finding `trunk` nearest means new work is being +built on top of it, never that `trunk` itself is being worked on. + +The graph does not distinguish the two cases: an unbookmarked or empty commit +above a bookmark may be the next commit of that change or the start of a new +one. Ask which it is when it is not clear, and ask before choosing where a fix +should land rather than after. Guessing wrong means squashing into a change that +may already be under review. + +Once the base is known, use it to scope commands to the current stack: + +```bash +jj --no-pager log -r '..@' +``` + +#### Managing the stack + +- **Fold a fix into an earlier change**: + `jj --no-pager squash --into [path]`. Follow-up fixes and + formatter reflows belong in the change that introduced the code, not in a + trailing "fixes" commit, unless that change has already been submitted. + Naming a path squashes only that part of the working copy, leaving unrelated + work in place. +- **Descriptions**: only one change in the stack needs a long description, the + one used as the pull request description. Every other change gets a short + one-line summary. Do not repeat the long text across the stack. diff --git a/.agents/skills/prek/SKILL.md b/.agents/skills/prek/SKILL.md index 2b862cfbfea0..fd444cf034d4 100644 --- a/.agents/skills/prek/SKILL.md +++ b/.agents/skills/prek/SKILL.md @@ -43,6 +43,21 @@ script: This script runs `prek` on all files that have changed between `trunk` and your current Jujutsu `@` change. +Note that the script always compares against `trunk`. If your change is based on +another bookmark rather than on `trunk`, the script also checks the files +changed by that underlying change, so a reported failure may not be in your own +work. + +## Hooks that rewrite files + +Some hooks, notably `clang-format` and `rumdl`, fix problems in place rather +than only reporting them. When they do, `prek` reports a failure and exits +non-zero even though the tree is now correct. + +Re-run `prek` after any failure that modified files, and treat the second, clean +run as the result. Review what it changed: a reflow is expected, but a content +change may not be what you intended. + ## Prek dependency errors > [!TIP] If `prek` fails with an error about resolving dependencies or security diff --git a/AGENTS.md b/AGENTS.md index 967b67734505..040fa542f416 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -20,3 +20,11 @@ contributing to the Carbon Language project. > [!IMPORTANT] Always use `bazelisk` instead of `bazel` for all commands in the > Carbon project. Refer to the > [Bazel usage skill](/.agents/skills/bazel/SKILL.md) for detailed instructions. + +## Version control + +> [!IMPORTANT] Never rewrite the history of a change that has been submitted as +> a pull request. Reviewers track a PR by its commits, and rewriting them +> discards their in-progress review. Ask before rewriting history in any case. +> Refer to the [Jujutsu (jj) usage skill](/.agents/skills/jj/SKILL.md) for +> details.