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

From: Jan Durovec <jan.durovec@gmail.com>
Date: Apr 19, 2016, 16:56 UTC
Message-ID: <CABEqOBxZkYTm7_m-Eeq-acN=Nse1vLGk8Gm44BihVGi27KaGiw@mail.gmail.com>
In-Reply-To: <xmqqfuuh35v5.fsf@gitster.mtv.corp.google.com>

Hi,

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)
* no space between function name and parentheses (see all functions
  in t/lib-git-p4.sh)
* 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?

Is there something else that I have missed in my enumeration?

Regards, Jan

On Tue, Apr 19, 2016 at 6:45 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 13 quoted lines

> Luke Diamand <luke@diamand.org> writes:
>
>>> I am not familiar with "Perforce jobs", but I assume that they are
>>> always named as "job" + small non-negative integer in a dense way
>>> and it is OK for this loop to always begin at 0 and immediately stop
>>> when job + num does not exist (i.e. if job7 is missing, it is
>>> guaranteed that we will not see job8).
>>
>> This is OK - P4 jobs have arbitrary names, but this code is just
>> extracting an array of them from the commit by index.
>
> Ah, thanks, that is what I was missing and this part of the code
> makes perfect sense to me now.