Re: [Outreachy] [PATCH v3] diff: do not show submodule with untracked files as "-dirty"

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 23, 2020, 18:55 UTC
Message-ID: <xmqqd018hivx.fsf@gitster.c.googlers.com>
In-Reply-To: <CAHjREB4egt95fCz6pzON2h5rVH-XvYKf1oGnyU-8gVL-U3WrmA@mail.gmail.com>

Sangeeta NB <sangunb09@gmail.com> writes:

Show 19 quoted lines

>> I found this change a "noise":
>
> Oh okay, Again sorry for the misunderstanding.
>
>>
>>         strvec_pushl(&cp.args, "status", "--porcelain=2", NULL);
>>         if (ignore_untracked)
>> -               strvec_push(&cp.args, "-uno");
>> +               strvec_push (&cp.args, "-uno");
>>
>> If it were going the other direction, "we fix coding style violation
>> while at it" may be a good justification to do so, but this
>> particular change (1) is not neeeded for the purpose of this patch,
>> and (2) is making the code worse by deviating from the coding
>> guideline.  Please drop it.
>>
> This part of the change was introduced because we had a failing test
> here[3]. There was some problem it getting both the flags propagated
> through ...

Are you talking about the new "else" clause added to the "if" statement we see above? I am not saying it is a "noise".

But look at what you did to the existing call to strvec_push() to add "-uno" shown above in the patch, i.e. the addition of space before the parenthesis. We cannot justify that change, can we? That's noise as far as I can see.