[v5,1/5] strftime: Add missing uses of L_ macro, etc. [BZ #23758]

  At first, make an unrelated changes for the consistency.

ChangeLog:

	[BZ #23758]
	* time/strftime_l.c (__strftime_internal): Add missing uses of L_
	macro, also add a missing space after the cast of _NL_CURRENT.
---
 time/strftime_l.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
  

Hello Tamuki Shoichi,

Thank you for the next version of your patches.

1. Please remove any reference to [BZ #23758] from this patch
because it is not related with the bug.  The changes are minor
and not visible for the users therefore they don't need any
Bugzilla report, documentation, etc.

2. Regarding the subject of this email, which is also the first
line of the commit message, I would write something like this:

"strftime: Consequently use the "L_" macro with character literals."

As always, I am not a native speaker so other people may provide
better hints.

6.01.2019 07:33 TAMUKI Shoichi <tamuki@linet.gr.jp> wrote:
> 
> At first, make an unrelated changes for the consistency.

If it is the part of the commit message then something like:

"Make unrelated changes for the consistency."

(The core problem is that "an" is incorrect for plural numbers.)

> 
> ChangeLog:
> 
> 	[BZ #23758]
> 	* time/strftime_l.c (__strftime_internal): Add missing uses of L_
> 	macro, also add a missing space after the cast of _NL_CURRENT.

Good but again, Bugzilla mention should be removed and "missing
uses" seems incorrect to me, "Add "L_" macros" or "Use "L_" macros"
is sufficient and seems correct to me.

I cut the patch body here because it is correct and trivial, also
I think it does not introduce any changes in the binary code but
it's good because it improves the readability of the source code.
Therefore I think it is OK to push it now with the changes above.
But due to the freeze period I'd like to hear "OK" from Siddhesh
therefore I'm adding CC: to him.

Regards,

Rafal
  

On 09/01/19 3:32 PM, Rafal Luzynski wrote:
> Hello Tamuki Shoichi,
> 
> Thank you for the next version of your patches.
> 
> 1. Please remove any reference to [BZ #23758] from this patch
> because it is not related with the bug.  The changes are minor
> and not visible for the users therefore they don't need any
> Bugzilla report, documentation, etc.
> 
> 2. Regarding the subject of this email, which is also the first
> line of the commit message, I would write something like this:
> 
> "strftime: Consequently use the "L_" macro with character literals."
> 
> As always, I am not a native speaker so other people may provide
> better hints.
> 
> 6.01.2019 07:33 TAMUKI Shoichi <tamuki@linet.gr.jp> wrote:
>>
>> At first, make an unrelated changes for the consistency.
> 
> If it is the part of the commit message then something like:
> 
> "Make unrelated changes for the consistency."
> 
> (The core problem is that "an" is incorrect for plural numbers.)
> 
>>
>> ChangeLog:
>>
>> 	[BZ #23758]
>> 	* time/strftime_l.c (__strftime_internal): Add missing uses of L_
>> 	macro, also add a missing space after the cast of _NL_CURRENT.
> 
> Good but again, Bugzilla mention should be removed and "missing
> uses" seems incorrect to me, "Add "L_" macros" or "Use "L_" macros"
> is sufficient and seems correct to me.
> 
> I cut the patch body here because it is correct and trivial, also
> I think it does not introduce any changes in the binary code but
> it's good because it improves the readability of the source code.
> Therefore I think it is OK to push it now with the changes above.
> But due to the freeze period I'd like to hear "OK" from Siddhesh
> therefore I'm adding CC: to him.

OK.

Siddhesh
  

9.01.2019 11:21 Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
> 
> On 09/01/19 3:32 PM, Rafal Luzynski wrote:
> > [...]
> > Therefore I think it is OK to push it now with the changes above.
> > But due to the freeze period I'd like to hear "OK" from Siddhesh
> > therefore I'm adding CC: to him.
> 
> OK.

This means: Tamuki Shoichi, please push this patch (only this first one)
with the changes as suggested above. [1]

Do you need an assistance for this?

Regards,

Rafal

[1] https://sourceware.org/ml/libc-alpha/2019-01/msg00193.html
  

Hello Rafal,

From: Rafal Luzynski <digitalfreak@lingonborough.com>
Subject: Re: [PATCH v5 1/5] strftime: Add missing uses of L_ macro, etc. [BZ #23758]
Date: Wed, 9 Jan 2019 11:02:25 +0100 (CET)

> 1. Please remove any reference to [BZ #23758] from this patch
> because it is not related with the bug.  The changes are minor
> and not visible for the users therefore they don't need any
> Bugzilla report, documentation, etc.

OK.  I will do that as an independent patch.

> 2. Regarding the subject of this email, which is also the first
> line of the commit message, I would write something like this:
> 
> "strftime: Consequently use the "L_" macro with character literals."
> 
> As always, I am not a native speaker so other people may provide
> better hints.

Thank you.

> > At first, make an unrelated changes for the consistency.
> 
> If it is the part of the commit message then something like:
> 
> "Make unrelated changes for the consistency."
> 
> (The core problem is that "an" is incorrect for plural numbers.)

Thank you.  Also, "At first" is also incorrect, should be "First" (or
"First of all").  But I will not add any of them this time because of
an independent patch.

> > ChangeLog:
> > 
> > 	[BZ #23758]
> > 	* time/strftime_l.c (__strftime_internal): Add missing uses of L_
> > 	macro, also add a missing space after the cast of _NL_CURRENT.
> 
> Good but again, Bugzilla mention should be removed and "missing
> uses" seems incorrect to me, "Add "L_" macros" or "Use "L_" macros"
> is sufficient and seems correct to me.

OK.  I will correct in the next patch.

Regards,
TAMUKI Shoichi
  

Hello Rafal,

From: Rafal Luzynski <digitalfreak@lingonborough.com>
Subject: Re: [PATCH v5 1/5] strftime: Add missing uses of L_ macro, etc. [BZ #23758]
Date: Wed, 9 Jan 2019 23:58:01 +0100 (CET)

> > > Therefore I think it is OK to push it now with the changes above.
> > > But due to the freeze period I'd like to hear "OK" from Siddhesh
> > > therefore I'm adding CC: to him.
> > 
> > OK.
> 
> This means: Tamuki Shoichi, please push this patch (only this first one)
> with the changes as suggested above. [1]
> 
> Do you need an assistance for this?

I have sent the latest patches reflecting your suggest.

If it does not matter, I will push the patch (first, only the first
one).  This is my first time to push to publish the changes upstream.
Please let me know if there is my sourceware username "tamuki".

Regards,
TAMUKI Shoichi
  

11.01.2019 05:54 TAMUKI Shoichi <tamuki@linet.gr.jp> wrote:
> 
> Hello Rafal,
> 
> From: Rafal Luzynski <digitalfreak@lingonborough.com>
> Subject: Re: [PATCH v5 1/5] strftime: Add missing uses of L_ macro, etc.
> [BZ #23758]
> Date: Wed, 9 Jan 2019 23:58:01 +0100 (CET)
> 
> > [...]
> > Do you need an assistance for this?
> 
> I have sent the latest patches reflecting your suggest.
> 
> If it does not matter, I will push the patch (first, only the first
> one).

Yes, only the first one, with unrelated changes.

> This is my first time to push to publish the changes upstream.
> Please let me know if there is my sourceware username "tamuki".

I am unable to verify this because I don't have any access to the
administrative resources of sourceware.  Other people should do it.

To other people: is there anybody able to assist Tamuki Shoichi
with his first commit to the main repo?  I'm afraid I am unable
to do it today.

Regards,

Rafal
  

On 11/01/19 10:24 AM, TAMUKI Shoichi wrote:
> If it does not matter, I will push the patch (first, only the first
> one).  This is my first time to push to publish the changes upstream.
> Please let me know if there is my sourceware username "tamuki".

Have you requested an account[1]?  If yes then you should have received 
confirmation and access details.  If not, Rafal should be able to commit 
for you.

Siddhesh

[1] https://sourceware.org/cgi-bin/pdw/ps_form.cgi