git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [RFC PATCH 0/3] Fixes to "diff --color-moved" MIN_BLOCK_LENGTH handling

From
Junio C Hamano <gitster@pobox.com>
Date
Aug 15, 2017, 17:07 UTC
Message-ID
<xmqqinhosqtc.fsf@gitster.mtv.corp.google.com>
In-Reply-To
<CAGZ79kYHW3bpexhiDnoNfyp=etBJ6nPhyLYR09+4jMpw25hR_A@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
Show 20 quoted lines
>> I see what I wrote can be misread, especially due to its lack of
>> ",instead", that I want to keep the broken one as-is, with neither
>> reroll nor fixup.  That is not what I meant.
>>
>>  - If you choose to squash so that the resulting history after the
>>    series graduates to 'master' will be simpler to read (due to lack
>>    of "oops, that was a mistake"), I do not mind a reroll.
>>
>>  - On the other hand, as the topic has been in 'next' for some time
>>    and presumably people tried it in their real daily work when
>>    needed, keeping what is queued as-is has a value---we have a
>>    fixed reference point that we can go back to to compare the code
>>    with and without the fix.
>>
>> I do not have a strong preference, but if I were asked to choose,
>> I'd choose the latter.
>
> We'll go with the latter then. Thanks!
>
> Other reasons for the latter that I want to add:

Yup, we are on the same page. You articulated what I meant in the "On the other hand" bullet point in a better way.

Even though we generally do not tolerate stupid mistakes and design errors to clutter the history if they are found early in the review process while the patches are still in flight, code that have been "in" for extended period of time and then found it has room for improvement is a different matter. There is a reason why we thought it was good enough initially, and there is a reason why we later found it needing improvement. Doing the latter as an incremental fix-up is a good way to leave records for both in our history.

And to make that kind of incremental refinement useful, it helps to keep the history clean from an initial attempt riddled with trivial issues that are found early in the review is also important.

Previous: Stefan BellerNext: Jonathan Tan
Message 12 of 30 in “Fixes to "diff --color-moved" MIN_BLOCK_LENGTH handling”
  1. 0/3 Fixes to "diff --color-moved" MIN_BLOCK_LENGTH handlingJonathan Tan, Aug 11, 2017
  2. 1/3 diff: avoid redundantly clearing a flagJonathan Tan, Aug 11, 2017
  3. Stefan BellerAug 14, 2017
  4. 2/3 diff: respect MIN_BLOCK_LENGTH for last blockJonathan Tan, Aug 11, 2017
  5. Stefan BellerAug 14, 2017
  6. 3/3 diff: check MIN_BLOCK_LENGTH at start of new blockJonathan Tan, Aug 11, 2017
  7. Stefan BellerAug 14, 2017
  8. Junio C HamanoAug 12, 2017
  9. Stefan BellerAug 14, 2017
  10. Junio C HamanoAug 14, 2017
  11. Stefan BellerAug 14, 2017
  12. Junio C HamanoAug 15, 2017
  13. 0/3 Fixes to "diff --color-moved" MIN_BLOCK_LENGTH handlingJonathan Tan, Aug 14, 2017
  14. 1/3 diff: avoid redundantly clearing a flagJonathan Tan, Aug 14, 2017
  15. 2/3 diff: respect MIN_BLOCK_LENGTH for last blockJonathan Tan, Aug 14, 2017
  16. 3/3 diff: check MIN_BLOCK_LENGTH at start of new blockJonathan Tan, Aug 14, 2017
  17. Stefan BellerAug 14, 2017
  18. 0/3 "diff --color-moved" with different heuristicJonathan Tan, Aug 14, 2017
  19. 1/3 diff: avoid redundantly clearing a flagJonathan Tan, Aug 14, 2017
  20. 2/3 diff: respect MIN_BLOCK_LENGTH for last blockJonathan Tan, Aug 14, 2017
  21. 3/3 diff: define block by number of non-space charsJonathan Tan, Aug 14, 2017
  22. Stefan BellerAug 15, 2017
  23. Junio C HamanoAug 15, 2017
  24. Stefan BellerAug 15, 2017
  25. Junio C HamanoAug 15, 2017
  26. 0/3 "diff --color-moved" with yet another heuristicJonathan Tan, Aug 16, 2017
  27. 1/3 diff: avoid redundantly clearing a flagJonathan Tan, Aug 16, 2017
  28. 2/3 diff: respect MIN_BLOCK_LENGTH for last blockJonathan Tan, Aug 16, 2017
  29. 3/3 diff: define block by number of alphanumeric charsJonathan Tan, Aug 16, 2017
  30. Stefan BellerAug 16, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.