Re: [PATCH 1/8] remote-bzr: fix export of utf-8 authors
- From
Felipe Contreras <felipe.contreras@gmail.com>
- Date
- Aug 28, 2013, 22:25 UTC
- Message-ID
- <CAMP44s0t71X8dhTVmeEoGEbgsUiVWsKYe5eYj=tCN0oB96cSqQ@mail.gmail.com>
- In-Reply-To
- <xmqq7gf58okw.fsf@gitster.dls.corp.google.com>
On Wed, Aug 28, 2013 at 4:58 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 24 quoted lines
> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes: > >> Felipe Contreras <felipe.contreras@gmail.com> writes: >> >>> On Wed, Aug 28, 2013 at 3:05 PM, Matthieu Moy >>> <Matthieu.Moy@grenoble-inp.fr> wrote: >>> >>>>> + bzr log | grep "^committer: " > ../actual >>>>> + ) && >>>>> + >>>>> + echo "committer: Grégoire <committer@example.com>" > 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@gmail.com> Date: Sun, 4 Aug 2013 19:27:51 -0500 Subject: [PATCH] remote-bzr: fix export of utf-8 authors
Reported-by: Joakim Verona <joakim@verona.se> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> --- 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