Message ID | 201901110446.AA04172@tamuki.linet.gr.jp |
---|---|
State | New |
Headers | show |
Series | [v6] strftime: Consequently use the "L_" macro with character literals | expand |
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
diff --git a/time/strftime_l.c b/time/strftime_l.c index 6919749c630..7ba4179de3e 100644 --- a/time/strftime_l.c +++ b/time/strftime_l.c @@ -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);