[v6] strftime: Consequently use the "L_" macro with character literals
Commit Message
Make unrelated changes for the consistency.
ChangeLog:
* time/strftime_l.c (__strftime_internal): Use "L_" macros, also add a
missing space after the cast of "_NL_CURRENT".
---
time/strftime_l.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
Comments
Hello,
Please add "." (a dot, a full stop) at the end of the first line
of the commit message which is also the subject of this email.
So it should be:
"strftime: Consequently use the "L_" macro with character literals."
11.01.2019 05:46 TAMUKI Shoichi <tamuki@linet.gr.jp> wrote:
>
> Make unrelated changes for the consistency.
I think we can remove this line. First, the word "unrelated" seems
unclear to me because it suggests we are in some context (unrelated
with what?) Additionally, it does not give any new information.
It is OK if the commit message consists of only one line if the patch
is trivial and the first line explains everything.
>
> ChangeLog:
>
> * time/strftime_l.c (__strftime_internal): Use "L_" macros, also add a
> missing space after the cast of "_NL_CURRENT".
Yes, this is perfect and it is correct to put this in the commit
message. Also, please remember that you should also put these two
lines in the ChangeLog file itself and only then push your change.
See how other ChangeLog entries look and follow the convention.
> ---
> time/strftime_l.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
> [...]
I don't comment the rest of your patch because I reviewed it before
and it looks correct.
Please apply the changes I suggest and commit and push your patch
to the main repository.
Regards,
Rafal
On Jan 11 2019, Rafal Luzynski <digitalfreak@lingonborough.com> wrote:
> Please add "." (a dot, a full stop) at the end of the first line
> of the commit message which is also the subject of this email.
A subject line should not end with a period.
Andreas.
* Andreas Schwab:
> On Jan 11 2019, Rafal Luzynski <digitalfreak@lingonborough.com> wrote:
>
>> Please add "." (a dot, a full stop) at the end of the first line
>> of the commit message which is also the subject of this email.
>
> A subject line should not end with a period.
We don't have a firm rule here. Items in ChangeLog entries should end
with periods, but that doesn't automatically apply to commit subject
lines.
Thanks,
Florian
I understood Siddhesh' suggestion [1] as the request for me to push
this patch and I have pushed it now with minor changes in the commit
message. I have given up on the requirement of the trailing dot in
the first line for now.
Regards,
Rafal
[1] https://sourceware.org/ml/libc-alpha/2019-01/msg00250.html
11.01.2019 14:56 Florian Weimer <fweimer@redhat.com> wrote:
>
> * Andreas Schwab:
>
> > On Jan 11 2019, Rafal Luzynski <digitalfreak@lingonborough.com> wrote:
> >
> >> Please add "." (a dot, a full stop) at the end of the first line
> >> of the commit message which is also the subject of this email.
> >
> > A subject line should not end with a period.
>
> We don't have a firm rule here. [...]
I don't remember where I saw that rule. All I can find now is that
examples in the Contribution Checklist [1] do end with a period.
I will update here if I find anything more.
I agree that a subject line should not end with a period but no
matter how weird it looks I always add it for the sake of the commit
message format.
Regards,
Rafal
[1]
https://sourceware.org/glibc/wiki/Contribution%20checklist#Contribution_Email_Subject_Line
Hello Rafal,
From: Rafal Luzynski <digitalfreak@lingonborough.com>
Subject: Re: [PATCH v6] strftime: Consequently use the "L_" macro with character literals
Date: Fri, 11 Jan 2019 13:01:54 +0100 (CET)
> > Make unrelated changes for the consistency.
>
> I think we can remove this line. First, the word "unrelated" seems
> unclear to me because it suggests we are in some context (unrelated
> with what?) Additionally, it does not give any new information.
> It is OK if the commit message consists of only one line if the patch
> is trivial and the first line explains everything.
I have no particular preference about this, so I agree.
Regards,
TAMUKI Shoichi
Hello Rafal,
From: Rafal Luzynski <digitalfreak@lingonborough.com>
Subject: Re: [PATCH v6] strftime: Consequently use the "L_" macro with character literals
Date: Sat, 12 Jan 2019 00:03:57 +0100 (CET)
> I understood Siddhesh' suggestion [1] as the request for me to push
> this patch and I have pushed it now with minor changes in the commit
> message. I have given up on the requirement of the trailing dot in
> the first line for now.
Thank you for pushing my patch with minor changes in the commit
message.
Yes, I prefer not to end the first line with a period, because it is a
title and titles do not end with a period.
There is not a firm rule in Glibc Wiki, indeed. On the other hand, it
commonly seems that the summary line of commit message should not end
with a period. (Googling "git commit summary period".)
Regards,
TAMUKI Shoichi
@@ -820,7 +820,7 @@ __strftime_internal (CHAR_T *s, size_t maxsize, const CHAR_T *format,
if (modifier == L_('O'))
goto bad_format;
#ifdef _NL_CURRENT
- if (! (modifier == 'E'
+ if (! (modifier == L_('E')
&& (*(subfmt =
(const CHAR_T *) _NL_CURRENT (LC_TIME,
NLW(ERA_D_T_FMT)))
@@ -917,7 +917,7 @@ __strftime_internal (CHAR_T *s, size_t maxsize, const CHAR_T *format,
#ifdef _NL_CURRENT
if (! (modifier == L_('E')
&& (*(subfmt =
- (const CHAR_T *)_NL_CURRENT (LC_TIME, NLW(ERA_D_FMT)))
+ (const CHAR_T *) _NL_CURRENT (LC_TIME, NLW(ERA_D_FMT)))
!= L_('\0'))))
subfmt = (const CHAR_T *) _NL_CURRENT (LC_TIME, NLW(D_FMT));
goto subformat;
@@ -1262,7 +1262,7 @@ __strftime_internal (CHAR_T *s, size_t maxsize, const CHAR_T *format,
DO_NUMBER (1, tp->tm_wday);
case L_('Y'):
- if (modifier == 'E')
+ if (modifier == L_('E'))
{
#if HAVE_STRUCT_ERA_ENTRY
struct era_entry *era = _nl_get_era_entry (tp HELPER_LOCALE_ARG);