Re: [PATCH v2] git-p4: add P4 jobs to git commit message

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 19, 2016, 17:47 UTC
Message-ID: <xmqq7fft32y9.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CABEqOBxZkYTm7_m-Eeq-acN=Nse1vLGk8Gm44BihVGi27KaGiw@mail.gmail.com>

Jan Durovec <jan.durovec@gmail.com> writes:

> given the fact that the rest of the code just follows existing
> source code style, i.e.
>
> * using %s not %d to add number to string (see git-p4.py:2301)

This one I do not care too deeply about, as formatting anything that can be formatted via '%s' could just be more Pythonic style, in which case "%s" is perfectly fine. It just didn't look kosher to my C trained eyes, that's all.

> * no space between function name and parentheses (see all functions
>   in t/lib-git-p4.sh)

I thought I said "Not a new issue, but..." to this one, and it appears that leaving <<- here-doc unindented, which is stupid as that shows the person who is writing the here-doc does not know what the dash s/he is writing means at all, is also not a new issue.

> * no tab when specifying in-line expected output (see t/t9826...)

> ...is there anything left to fix in this patch or is it good as is?
>
> I.e. would you prefer me to change the code mentioned above at the cost
> of code style consistency?

Not really.

If you really want to know the preference, we prefer a preliminary clean-up patch to correct existing style issues, followed by a new feature patch that builds on the cleaned up codebase.