Re: [PATCH 3/4] {fast-export,transport-helper}: style cleanups
- From
Felipe Contreras <felipe.contreras@gmail.com>
- Date
- May 16, 2013, 09:23 UTC
- Message-ID
- <CAMP44s3J8YpULYenDoYhyRJXSXL3b8-vpMbW2c4LcjK43Xidng@mail.gmail.com>
- In-Reply-To
- <CAMP44s1HDp+ojGK0UhKHF=1iDu5_E9Z0VrK-JtMked1mtH_2gQ@mail.gmail.com>
On Thu, May 9, 2013 at 6:12 PM, Felipe Contreras <felipe.contreras@gmail.com> wrote:
Show 37 quoted lines
> On Thu, May 9, 2013 at 6:09 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> John Szakmeister <john@szakmeister.net> writes:
>>
>>> On Wed, May 8, 2013 at 9:16 PM, Felipe Contreras
>>> <felipe.contreras@gmail.com> wrote:
>>>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>>>> ---
>>>> builtin/fast-export.c | 24 ++++++++++++------------
>>>> 1 file changed, 12 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/builtin/fast-export.c b/builtin/fast-export.c
>>>> index d60d675..8091354 100644
>>>> --- a/builtin/fast-export.c
>>>> +++ b/builtin/fast-export.c
>>>> @@ -135,7 +135,7 @@ static void export_blob(const unsigned char *sha1)
>>> [snip]
>>>> @@ -289,13 +289,13 @@ static void handle_commit(struct commit *commit, struct rev_info *rev)
>>>> parse_commit(commit);
>>>> author = strstr(commit->buffer, "\nauthor ");
>>>> if (!author)
>>>> - die ("Could not find author in commit %s",
>>>> + die("Could not find author in commit %s",
>>>> sha1_to_hex(commit->object.sha1));
>>>
>>> It looks like your simple replace didn't account for calls with
>>> multiple lines. Now the remaining lines don't line up.
>>> :-) There's several more places like this in the patch.
>>
>> Good eyes.
>>
>> Matching the coding-style to have no SP between function name and
>> its argument list is just as important as matching the indentation
>> style used in the project; trading one breakage with another does
>> not make much sense.
>
> Where exactly in Documentation/CodingGuidelines is the "indentation
> style" used in the project specified that is being violated?I find it extremely annoying that an obviously correct patch is not merged because it's not conforming to a non-existing coding-style guideline that not even the Linux project follows. I've sent many patches where I change the alignment from cino=(0, to cino=(2s, and the get applied because if it's not mentioned in Documentation/CodingStyle, it cannot be used as a reason for rejection.
I fixed the style so it conforms to Documentation/CodingGuidelines, and nowhere in there is the open parenthesis alignment mentioned, so using that as a reason to reject this patch is a mistake in my opinion.
If you prefer the code to not follow Documentation/CodingGuidelines, so be it. I'm not going to work on this patch any more.
-- Felipe Contreras