cryptogera/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-03-02 01:36:13 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#34656: doc: clarify confusing git range-diff add/delete output

Browse Source 
45133c589a970390bc10e4db4f7d9921dbaa0832 doc: clarify `git range-diff` add/delete output (Lőrinc)

Pull request description:

  ### Problem
  Range diffs in git are useful after PR rebases, but it has an easy-to-misread failure mode: if it cannot match a commit between the old and new ranges, it will show the old commit as removed (<) and the new commit as added (>), without showing any patch contents for that commit.
  It can look like there were no code changes when in reality the commit was just treated as unrelated and needs full re-review.

  ### Example

  ```bash
  git fetch upstream ff338fdb53a66ab40a36e1277e7371941fc89840 dd76338a57b9b1169ac27f7b783d6d0d4c6e38ab
  git range-diff ff338fdb53a6...dd76338a57b9
  ```

  This produced output like:
  ```patch
  1:  0ca4295f2e = 93:  139aa4b27e bench: add on-disk `HaveInputs` benchmark
  2:  4b32181dbb <  -:  ---------- test: add `HaveInputs` call-path unit tests
  -:  ---------- > 94:  277c57f0c5 test: add `HaveInputs` call-path unit tests
  3:  8c57687f86 ! 95:  c0c94ec986 dbwrapper: have `Read` and `Exists` reuse `ReadRaw`
  @@ Metadata
  ## Commit message ##
     dbwrapper: have `Read` and `Exists` reuse `ReadRaw`

  -    `ExistsImpl` was removed since it duplicates `CDBWrapper::ReadImpl` (except that it copies the resulting string on success, but that will be needed for caching anyway).
  +    `ExistsImpl` was removed since it duplicates `CDBWrapper::ReadImpl`.
  ```

  Even though the subject matches, there is no diff shown because the commits did not match - the reviewer could think that only the commit message was changed.
  This should be treated as **unmatched** rather than **unchanged**.
  If you expected a match, you can try increasing the search effort:
  ```bash
  git range-diff --creation-factor=95 ff338fdb53a6...dd76338a57b9
  ```
  which would show for example:
  ```patch
  1:  0ca4295f2e = 93:  139aa4b27e bench: add on-disk `HaveInputs` benchmark
  2:  4b32181dbb ! 94:  277c57f0c5 test: add `HaveInputs` call-path unit tests
  @@ Commit message

       The tests document that `HaveInputs()` consults the cache first and that a cache miss pulls from the backing view via `GetCoin()`.

  +    Co-authored-by: Novo <eunovo9@gmail.com>
  +
    ## src/test/coins_tests.cpp ##
   @@ src/test/coins_tests.cpp: BOOST_FIXTURE_TEST_CASE(ccoins_flush_behavior, FlushTest)
        }
    }

  -+BOOST_AUTO_TEST_CASE(ccoins_haveinputs_cache_miss_uses_base_getcoin)
  ++BOOST_AUTO_TEST_CASE(ccoins_cache_behavior)
  ```

  ### Fix
  This PR updates `doc/productivity.md` to raise awareness and document this pitfall and mentions `--creation-factor` as a knob to try when the output seems unexpectedly empty.

ACKs for top commit:
  maflcko:
    review ACK 45133c589a970390bc10e4db4f7d9921dbaa0832 🏦
  Sjors:
    ACK 45133c589a970390bc10e4db4f7d9921dbaa0832
  rkrux:
    crACK 45133c5
  sedited:
    ACK 45133c589a970390bc10e4db4f7d9921dbaa0832

Tree-SHA512: 52dcf6db51425a3ac9789627f80233fb1e3437f7a351acf4a761504d9917837aa1ff8c964605a842ee099fae9842946784f7603f9bffa7051429b2f04b7900be
This commit is contained in:
merge-script 2026-02-27 10:21:39 +00:00
commit 3c7b0f97e0
No known key found for this signature in database
GPG Key ID: 2EEB9F5CC09526C1
1 changed files with 12 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
doc/productivity.md
View File

@ -191,7 +191,12 @@ Then a simple `git pr 12345` will fetch and check out that pr from upstream.
### Diff the diffs with `git range-diff`
It is very common for contributors to rebase their pull requests, or make changes to commits (perhaps in response to review) that are not at the head of their branch. This poses a problem for reviewers as when the contributor force pushes, the reviewer is no longer sure that his previous reviews of commits are still valid (as the commit hashes can now be different even though the diff is semantically the same). [git range-diff](https://git-scm.com/docs/git-range-diff) (Git >= 2.19) can help solve this problem by diffing the diffs.
It is very common for contributors to rebase their pull requests, or make changes to commits (perhaps in response to review) that are not at the head of their branch. This poses a problem for reviewers as when the contributor force pushes, the reviewer is no longer sure that their previous reviews of commits are still valid (as the commit hashes can now be different even though the diff is semantically the same). [git range-diff](https://git-scm.com/docs/git-range-diff) (Git >= 2.19) can help solve this problem by diffing the diffs.
> [!NOTE]
> If `git range-diff` cannot match a commit in the old range to a commit in the new range, it will show it as "removed" (`<`) and "added" (`>`), without showing the patch contents.
> This does not mean there were no code changes.
> It means the commit was considered unrelated, and should be reviewed in full like a new commit.
For example, to identify the differences between your previously reviewed diffs P1-5, and the new diffs P1-2,N3-4 as illustrated below:
```
@ -207,6 +212,12 @@ You can do:
git range-diff master previously-reviewed-head new-head
```
If you expected `git range-diff` to match a commit, but it shows it as a deletion and an addition, try re-running with a higher creation factor:
```sh
git range-diff --creation-factor=95 <old_range> <new_range>
```
Note that `git range-diff` also works for rebases:
```