On Wed, Jun 11, 2014 at 04:09:17AM -0400, Eric Sunshine wrote:
> On Mon, Jun 9, 2014 at 6:19 PM, Jeremiah Mahler <> wrote:
> > Version 2 of the patch set to add strbuf_set operations.
> >
> > Includes suggestions from Eric Sunshine [1]:
> >
> > [...snip...]
> >
> Food for thought: This patch series has now likely entered the
> territory of unnecessary code churn. Quoting from
> Documentation/CodingGuidelines:
>     Fixing style violations while working on a real change as a
>     preparatory clean-up step is good, but otherwise avoid useless
>     code churn for the sake of conforming to the style.
>     "Once it _is_ in the tree, it's not really worth the patch noise
>     to go and fix it up."
>     Cf.
> Rewriting code merely for the sake of replacing strbuf_reset() +
> strbuf_add() with strbuf_set(), while not exactly a style cleanup,
> falls into the same camp. (The patch which introduces strbuf_set(),
> however, is not churn.) Each change you make can potentially conflict
> with other topics already in-flight (see [1], for example), thus
> making more work for Junio.
> Also, these sorts of apparently innocuous "mechanical" changes can
> have hidden costs [2], so it's useful to weigh carefully how lengthy
> you want to make this patch series.
> Having said that, patch 4/19, particularly the changes to
> builtin/remote.c:mv(), in which multiple strbufs are reused
> repeatedly, does seem to make the code a bit easier to read and likely
> reduces cognitive load.
> [1]: 
> [2]: 

I did not realize this "code churn" aspect at the time but it makes
sense now.  These "mechanical" changes can be more trouble than they are

I will try to re-focus my changes to only those that have a substantial

Jeremiah Mahler
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to
More majordomo info at

Reply via email to