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

From: Junio C Hamano <gitster@pobox.com>
To: Sangeeta NB <sangunb09@gmail.com>
Cc: Git List <git@vger.kernel.org>,
	Phillip Wood <phillip.wood123@gmail.com>,
	Kaartic Sivaraam <kaartic.sivaraam@gmail.com>,
	Eric Sunshine <sunshine@sunshineco.com>
Subject: Re: [Outreachy] [PATCH v3] diff: do not show submodule with untracked files as "-dirty"
Date: Fri, 23 Oct 2020 11:55:30 -0700	[thread overview]
Message-ID: <xmqqd018hivx.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <CAHjREB4egt95fCz6pzON2h5rVH-XvYKf1oGnyU-8gVL-U3WrmA@mail.gmail.com> (Sangeeta NB's message of "Fri, 23 Oct 2020 23:47:20 +0530")

Sangeeta NB <sangunb09@gmail.com> writes:

>> 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.