Re: [PATCH 1/8] remote-bzr: fix export of utf-8 authors

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Aug 28, 2013 at 4:58 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Matthieu Moy <Matthieu.Moy@xxxxxxxxxxxxxxx> writes:
>
>> Felipe Contreras <felipe.contreras@xxxxxxxxx> writes:
>>
>>> On Wed, Aug 28, 2013 at 3:05 PM, Matthieu Moy
>>> <Matthieu.Moy@xxxxxxxxxxxxxxx> wrote:
>>>
>>>>> +     bzr log | grep "^committer: " > ../actual
>>>>> +     ) &&
>>>>> +
>>>>> +     echo "committer: Grégoire <committer@xxxxxxxxxxx>" > expected &&
>>>>
>>>> Git's source code usually says >../actual and >expected, without space
>>>> after '>'.
>>>
>>> Not that usually:
>>>
>>> % git grep ' > ' t/*.sh | wc -l
>>> 1943
>>
>> Do I really need to quote the paragraph in CodingGuidelines?
>
> Existing violations are not an excuse to make things worse by adding
> more.

1)

By definition following a guideline is not mandatory:
https://en.wikipedia.org/wiki/Guideline

Violation is a too strong word for not following something that is not
mandatory in the first place; deviation is a more appropriate word.

2)

This code is part of the 'contrib' area, and part of the tests, most
of the code in this area doesn't even have tests.

3)

Blindly following guidelines is not a good idea.

cmd >out
cmd >out 2>err
if (n > 1)
cmd1 | cmd2

Clearly, those are inconsistent.

cmd > out
cmd > out 2> err
if (n > 1)
cmd1 | cmd2

Those are not.

Fortunately since these rules are only guidelines, there's no need to
engage in bike-sheding argumentation if the code deviates from
guideline, specially for this insignificantly trivial issue.

Blocking a patch that would be obviously advantageous for the users on
the basis of a single white-space on the tests which aren't even run
by default is simply stupid.

> I think with these comments we can expect a reroll coming,
> and it should be trivial for any contributor to fix it while at it.

I already send the update, I do not see the need for any more changes,
so no more changes will come from me unless there is valid feedback.

If it makes you happier, apply this:

>From 7db40fa6b2780f9b5787c7d38f0d2dac01b175d3 Mon Sep 17 00:00:00 2001
From: Felipe Contreras <felipe.contreras@xxxxxxxxx>
Date: Sun, 4 Aug 2013 19:27:51 -0500
Subject: [PATCH] remote-bzr: fix export of utf-8 authors

Reported-by: Joakim Verona <joakim@xxxxxxxxx>
Signed-off-by: Felipe Contreras <felipe.contreras@xxxxxxxxx>
---
 contrib/remote-helpers/git-remote-bzr | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/remote-helpers/git-remote-bzr
b/contrib/remote-helpers/git-remote-bzr
index c3a3cac..08b0b61 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -168,6 +168,7 @@ class Parser:
         if not m:
             return None
         _, name, email, date, tz = m.groups()
+        name = name.decode('utf-8')
         committer = '%s <%s>' % (name, email)
         tz = int(tz)
         tz = ((tz / 100) * 3600) + ((tz % 100) * 60)
-- 
1.8.4-fc-3-g64bc480

There, there is no more test GUIDEline "violation".

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]