Thread (1 message) 1 message, 1 author, 2017-08-15

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

From: Junio C Hamano <hidden>
Date: 2017-08-15 17:07:26

Stefan Beller [off-list ref] writes:
quoted
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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help