From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Wed, 11 Sep 2019 10:51:21 -0700 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 1DB3A10DCC82; Wed, 11 Sep 2019 17:51:21 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-127.ams2.redhat.com [10.36.116.127]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3A3CC5C226; Wed, 11 Sep 2019 17:51:16 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH edk2-CCSS 0/3] Coding Standards: add rule for documenting spurious variable assignments To: devel@edk2.groups.io, ryszard.knop@linux.intel.com, leif.lindholm@linaro.org Cc: Andrew Fish , Michael D Kinney , Rebecca Cran , Philippe Mathieu-Daude References: <20190905183820.10312-1-lersek@redhat.com> <20190906122623.GS29255@bivouac.eciton.net> <54fb3783-7589-feff-e446-1e592686d7d2@redhat.com> <20190910153304.GD15201@bivouac.eciton.net> <57976eb87976eba0e18f70726ad1813f440e7acf.camel@linux.intel.com> From: "Laszlo Ersek" Message-ID: <4fcb4e8d-94e6-430d-dac4-450b166c7f1e@redhat.com> Date: Wed, 11 Sep 2019 19:51:15 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <57976eb87976eba0e18f70726ad1813f440e7acf.camel@linux.intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.6.2 (mx1.redhat.com [10.5.110.64]); Wed, 11 Sep 2019 17:51:21 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 09/10/19 17:44, Ryszard Knop wrote: > On Tue, 2019-09-10 at 16:33 +0100, Leif Lindholm wrote: >> On Mon, Sep 09, 2019 at 02:35:15PM +0200, Laszlo Ersek wrote: >>> On 09/06/19 14:26, Leif Lindholm wrote: >>>> On Thu, Sep 05, 2019 at 08:38:17PM +0200, Laszlo Ersek wrote: >>>>> Repo: >>>>> https://github.com/lersek/edk2-CCodingStandardsSpecification.git >>>>> Branch: spurious_assign_bz_607 >>>>> >>>>> HTML-rendered views of the modified pages: >>>>> - >>>>> https://lersek.gitbooks.io/laszlo-s-fork-of-the-edk-ii-c-coding-standards-sp/content/v/spurious_assign_bz_607 >>>>> - >>>>> https://lersek.gitbooks.io/laszlo-s-fork-of-the-edk-ii-c-coding-standards-sp/content/v/spurious_assign_bz_607/6_documenting_software/62_comments.html >>>>> - >>>>> https://lersek.gitbooks.io/laszlo-s-fork-of-the-edk-ii-c-coding-standards-sp/content/v/spurious_assign_bz_607/6_documenting_software/64_what_you_must_comment.html >>>>> >>>>> The first two patches are cleanups for things that popped up in >>>>> the >>>>> discussion in < >>>>> https://bugzilla.tianocore.org/show_bug.cgi?id=607>;. >>>>> >>>>> The third patch is the one fixing the BZ. >>>> >>>> For 1 and 2, >>>> Reviewed-by: Leif Lindholm >>>> >>>> For 3, I see no issue with it, but I do feel tempted by Phil's >>>> input >>>> of using explicit macros (obviating the need for specific >>>> comment). >>>> I seem to recall back in the mists of time we considered >>>> something >>>> similar. >>> >>> Yes, I remember similarly. >>> >>>> Vaguely. Am I misremembering, or did we disount that option? >>> >>> Phil's current recommendation is what I would have preferred back >>> then, >>> but it was rejected, as far as I recall. If I remember correctly, >>> most >>> developers preferred naked NULLs / zeroes. I insisted on the >>> comment as >>> a fallback / compromise, so that we'd have at least some visual >>> cue. >> >> I'm not even sure I wasn't one of the people opposed to it then. >> But if I was, I would appear to have changed my mind. >> >>> I could be mis-remembering; we can restart that discussion if now >>> the >>> macros are preferred. >> >> I would be all for that. > > If my 2 cents are worth anything, that'd be preferred by some folks in > my team too. Although something shorter like "UNINITIALIZED_INT/PTR" > would be nicer, IMO. Both work of course. Thanks everyone for the feedback thus far on this series. It looks like I could go ahead and push the patches, minimally for bringing the CCSS in closer sync with reality -- and then we could improve incrementally, for example with macros. But, before I push the set, I'd really like hear Mike's opinion too -- I vaguely recall he was active in the original discussion. I wouldn't like to back out the patches in case Mike rejected them retroactively. I believe Mike will have a bit of an email backlog to process ;) so I'll wait some more in this thread. Thanks! Laszlo >> However, I see no reason why we shouldn't document the current >> process >> in the meantime, so for 3/3 also: >> Reviewed-by: Leif Lindholm >> >> Best Regards, >> >> Leif >> >>> Thanks, >>> Laszlo >>> >>>> Regards, >>>> >>>> Leif >>>> >>>>> Thanks, >>>>> Laszlo >>>>> >>>>> Cc: Andrew Fish >>>>> Cc: Leif Lindholm >>>>> Cc: Michael D Kinney >>>>> Cc: Rebecca Cran >>>>> >>>>> Laszlo Ersek (3): >>>>> comments: remove "Horror Vacui" rule >>>>> comments: restrict and clarify applicability of "/*" comments >>>>> must comment: add rule for documenting spurious variable >>>>> assignments >>>>> >>>>> 6_documenting_software/62_comments.md | 20 +----- >>>>> ---- >>>>> 6_documenting_software/64_what_you_must_comment.md | 39 >>>>> ++++++++++++++++++++ >>>>> README.md | 1 + >>>>> 3 files changed, 42 insertions(+), 18 deletions(-) >>>>> >>>>> -- >>>>> 2.19.1.3.g30247aa5d201 >>>>> >> >> >> > > > >