mirror of
https://github.com/carbon-language/carbon-lang.git
synced 2026-09-24 13:50:10 +01:00
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
This commit is contained in:
@@ -42,3 +42,61 @@ blocking or waiting for terminal paging.
|
|||||||
and `jj new`.
|
and `jj new`.
|
||||||
- **Abandon/discard current changes**: `jj --no-pager abandon`
|
- **Abandon/discard current changes**: `jj --no-pager abandon`
|
||||||
- **Rebase current change onto trunk**: `jj --no-pager rebase -o trunk`
|
- **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 '<base-bookmark>..@'
|
||||||
|
```
|
||||||
|
|
||||||
|
#### Managing the stack
|
||||||
|
|
||||||
|
- **Fold a fix into an earlier change**:
|
||||||
|
`jj --no-pager squash --into <change-id> [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.
|
||||||
|
|||||||
@@ -43,6 +43,21 @@ script:
|
|||||||
This script runs `prek` on all files that have changed between `trunk` and your
|
This script runs `prek` on all files that have changed between `trunk` and your
|
||||||
current Jujutsu `@` change.
|
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
|
## Prek dependency errors
|
||||||
|
|
||||||
> [!TIP] If `prek` fails with an error about resolving dependencies or security
|
> [!TIP] If `prek` fails with an error about resolving dependencies or security
|
||||||
|
|||||||
@@ -20,3 +20,11 @@ contributing to the Carbon Language project.
|
|||||||
> [!IMPORTANT] Always use `bazelisk` instead of `bazel` for all commands in the
|
> [!IMPORTANT] Always use `bazelisk` instead of `bazel` for all commands in the
|
||||||
> Carbon project. Refer to the
|
> Carbon project. Refer to the
|
||||||
> [Bazel usage skill](/.agents/skills/bazel/SKILL.md) for detailed instructions.
|
> [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.
|
||||||
|
|||||||
Reference in New Issue
Block a user