Re: [PATCH] symbolic-ref: trivial style fix

[Felipe Contreras <felipe.contreras@gmail.com>]

Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> > ---
> 
> Let's do something like this instead.
> 
>  - We usually refrain from making such a tree-wide change in order
>    to avoid unnecessary conflicts with other "real work" patches,
>    but the end result does not have a potentially cumbersome
>    tree-wide impact.

You might want to change that phrasing to "we usually give low priority". Sure,
this is low priority, but this is something has to be done sooner or later, if
we don't the code-style will be forever inconsistent.

If there is a conflict it would be sensible to delay the change for another
release perhaps, it's perfectly sensible to ask the submitter to set another
version, and hopefully there would be no conflict.

Usually resolving the conflict is trivial, but there's a lot of options.

Refraining from making a code-style fix is not ideal. We want those. It's only
a matter of how to implement them.

>  - We also tend to frown upon an "I fixed it here only because I
>    happened to notice this one, there may be others but it is up to
>    the readers to see if there are other instances" half-assed code
>    churn.

If you have more general patch, sure, but would you really reject a patch that
fixes one thing, because it doesn't fix all similar things at the same time?

> The point of the proposed log message is to tell readers that this
> is a tree-wide clean-up that is worth applying.
> 
> -- >8 --
> From: Felipe Contreras <felipe.contreras@gmail.com>
> Subject: C: have space around && and || operators

Saying this patch is from me is not really accurate, it's based on a patch by
me, or it was reported by me, but it's not really from me. I don't have a
problem taking the credit though, as probably half the change is finding out
there's a problem, just saying.

-- 
Felipe Contreras