This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] ja_JP locale: Fix the offset in era-string for Taisho gan-nen [BZ #24162]


23.02.2019 o 08:29 TAMUKI Shoichi <tamuki@linet.gr.jp> napisał(a):
> 
> 
> Hello Rafal-san,
> 
> Sorry to be late replying.  Thank you for the detailed review on this
> patch.
> 
> From: Rafal Luzynski <digitalfreak@lingonborough.com>
> Subject: Re: [PATCH] ja_JP locale: Fix the offset in era-string for Taisho
> gan-nen [BZ #24162]
> Date: Tue, 19 Feb 2019 00:27:36 +0100 (CET)
> 
> > [...]
> > Could you please shorten the first line of your patch, which is also
> > the subject line of your email?  If it was only 1 character shorter
> > it would fit in one line when listing with "git log --oneline" on
> > a traditional 80-columns terminal.  I think that if you remove the
> > word "locale" it is OK.  No problem if you don't want to change it,
> > some people do not keep the 72 columns limit and nobody complains.
> 
> OK.  I will remove the word "locale" with 's/ locale//'.  When listing
> with "git log --oneline" on a traditional 80-columns terminal, the
> commit message summary should be within 72-colmuns with the first 7
> characters of the commit hash (and 1 space).  If there is a bugzilla
> associated with the patch ([BZ #XXXXX]), we need to deduct 12 more
> columns.  So, we need to describe the commit message summary within
> 60-columns.

Thank you.

> [...]
> > > ChangeLog:
> > > 
> > > 	[BZ #24162]
> > > 	* localedata/locales/ja_JP (LC_TIME): The offset in era-string format
> > 
> > Somebody fix me if I'm wrong but I usually write here:
> > 
> > 	* localedata/locales/ja_JP (era):
> 
> I checked with the following command and confirmed that both are
> commonly used.
> 
> $ git log | grep "^    "$'\t'"\* localedata/locales/"

Sure.  ChangeLog guidelines are ambiguous so you may consider both
as correct.  What I write here is my personal opinion, my may agree
with it or disagree.  I do not discuss the body of your patch because
it is correct, nothing I can add.

Also, I forgot to say, I think you wanted to mention the reporter
in your ChangeLog.  Feel free to mention.  That we don't officially
use "Reported-by:" tag does not mean you can't just mention who found
the bug.

> [...]
> > Also I think that the issue is important and this patch does not depend
> > on other patches so it is also OK to backport it to old stable branches.
> > However, I'd like to hear an opinion of other maintainers about it.
> 
> As you are discussing here backporting, I think it is better to
> backport to 2.29 only.

Yes, we have all agreed on this.

Please see how other backported patches look and format yours the same.
Hint 1: use "git cherry-pick -x" to copy your commit from master to your
local copy of 2.29 branch.  Hint 2: You should update NEWS file in the
branch manually.  Yes, lots of people forget it but please do it.
Hint 3: Most likely you will have a conflict in the ChangeLog, you
will have to resolve it manually.

Thank you for your work on this patch.

Regards,

Rafal


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]