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

From: Jan Durovec <jan.durovec@gmail.com>
Date: Apr 19, 2016, 18:04 UTC
Message-ID: <CABEqOBwqW+BO4rtOx4ax35VacE4RZhpo_1pbqzTP_EGiSWasiQ@mail.gmail.com>
In-Reply-To: <xmqq7fft32y9.fsf@gitster.mtv.corp.google.com>

Would it be acceptable the other way around? I.e. this patch followed by the one that fixes code style (once this gets merged)?

Reason being that I don't know how to use submitGit to generate a patch against a state that is not already in git repo (ie. based on another patch).

In the following patch I'll
* add spaces before () for functions in t/lib-git-p4.sh
* remove name local variable in p4_add_job/user in t/lib-git-p4.sh
* fix t/t98* leading tabs where <<- is used

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

Show 33 quoted lines

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