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 v6] strftime: Consequently use the "L_" macro with character literals


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


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