Skip to content

bisect: --find-good: fix the docs - #10015

Open
badp wants to merge 1 commit into
jj-vcs:mainfrom
badp:bisect/find-good-docs
Open

bisect: --find-good: fix the docs#10015
badp wants to merge 1 commit into
jj-vcs:mainfrom
badp:bisect/find-good-docs

Conversation

@badp

@badp badp commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Looking at test_bisect_run_with_args, fake-bisector is being called with --require-file=c (= exit code 0 iff the file c is present), but with --find-good "status 0 means bad and other non-zero statuses mean good" (or so claims jj bisect run --help).

So, when evaluating commit c adding file c, fake-bisector finds the file c, has an exit code 0, and that should make the commit bad, not good.

However, the overall behavior of jj is self-consistent: jj calls the commits with the file 'good," the rest of the commits "bad," then declares that the comment that added the file was the first "good" commit. And that's what you'd expect to see from the incantation: jj bisect run --find-good fake-bisector --require-file=c should behave like it does in the test.

The bug, then, is in the documentation. It's not true that --find-good inverts how COMMAND is interpreted: COMMAND is still expected to have exit codes 0 for good commits, and non-zero for bad commits. It's the expectations about the edges of the range that are flipped.

This commit fixes the docs accordingly.

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (cli/src/config-schema.json)
  • I have added/updated tests to cover my changes
  • I fully understand the code that I am submitting (what it does,
    how it works, how it's organized), including any code drafted by an LLM.
  • For any prose generated by an LLM, I have proof-read and copy-edited with
    an eye towards deleting anything that is irrelevant, clarifying anything
    that is confusing, and adding details that are relevant. This includes,
    for example, commit descriptions, PR descriptions, and code comments.

@badp
badp requested a review from a team as a code owner August 19, 2026 21:29
@badp badp mentioned this pull request Aug 19, 2026
6 tasks
Comment thread cli/src/commands/bisect/run.rs Outdated
Comment thread cli/src/commands/bisect/run.rs Outdated
@badp
badp force-pushed the bisect/find-good-docs branch 5 times, most recently from ed1b18a to f120e0b Compare August 20, 2026 22:31
Comment thread cli/src/commands/bisect/run.rs Outdated
Comment thread cli/src/commands/bisect/run.rs Outdated
@badp
badp force-pushed the bisect/find-good-docs branch from f120e0b to ce4b6e8 Compare August 26, 2026 21:27
Looking at test_bisect_run_with_args, fake-bisector is being called with 
--require-file=c (= exit code 0 iff the file c is present), but with 
--find-good "status 0 means bad and other non-zero statuses mean good" 
(or so claims `jj bisect run --help`).

So, when evaluating commit c adding file c, fake-bisector finds the file 
c, has an exit code 0, and that should make the commit bad, not good.

However, the overall behavior of `jj` is self-consistent: jj calls the 
commits with the file 'good," the rest of the commits "bad," then 
declares that the comment that added the file was the first "good" 
commit. And that's what you'd expect to see from the incantation: `jj 
bisect run --find-good fake-bisector --require-file=c` SHOULD behave 
like it does in the test.

The bug, then, is in the documentation. It's not true that 
`--find-good` inverts how COMMAND is interpreted: COMMAND is still 
expected to have exit codes 0 for good commits, and non-zero for bad 
commits. It's the expectations about the edges of the range that are 
flipped.

This commit fixes the docs accordingly.
@badp
badp force-pushed the bisect/find-good-docs branch from ce4b6e8 to 865f070 Compare August 26, 2026 21:29

@josephlou5 josephlou5 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but still needs maintainer review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants