This is the mail archive of the 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 v5 1/5] strftime: Add missing uses of L_ macro, etc. [BZ #23758]

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 <> 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.



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