From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 88589 invoked by alias); 9 Jan 2019 10:08:21 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 88577 invoked by uid 89); 9 Jan 2019 10:08:20 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=0.5 required=5.0 tests=BAYES_05,KAM_LAZY_DOMAIN_SECURITY,RCVD_IN_DNSWL_NONE autolearn=no version=3.3.2 spammy=H*x:Mailer, H*UA:Mailer, HImportance:Medium, H*x:Open-Xchange X-HELO: shared-ano163.rev.nazwa.pl X-Spam-Score: 0 Date: Wed, 09 Jan 2019 10:08:00 -0000 From: Rafal Luzynski To: TAMUKI Shoichi , libc-alpha@sourceware.org Cc: Siddhesh Poyarekar Message-ID: <553127895.95059.1547028145705@poczta.nazwa.pl> In-Reply-To: <201901060633.AA04157@tamuki.linet.gr.jp> References: <201901060628.AA04156@tamuki.linet.gr.jp> <201901060633.AA04157@tamuki.linet.gr.jp> Subject: Re: [PATCH v5 1/5] strftime: Add missing uses of L_ macro, etc. [BZ #23758] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2019-01/txt/msg00193.txt.bz2 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. Regards, Rafal