Series Reviewed-by: Michael D Kinney <michael.d.kin...@intel.com> I also agree that the macros would be cleaner, easy to review, and and fewer lines of code without the comment block. If I objected previously, then I have also changed my mind. I agree we can go ahead and push the series in its current form and continue the discussion on the macros.
Mike > -----Original Message----- > From: Laszlo Ersek <ler...@redhat.com> > Sent: Wednesday, September 11, 2019 10:51 AM > To: devel@edk2.groups.io; ryszard.k...@linux.intel.com; > leif.lindh...@linaro.org > Cc: Andrew Fish <af...@apple.com>; Kinney, Michael D > <michael.d.kin...@intel.com>; Rebecca Cran > <rebe...@bsdio.com>; Philippe Mathieu-Daude > <phi...@redhat.com> > Subject: Re: [edk2-devel] [PATCH edk2-CCSS 0/3] Coding > Standards: add rule for documenting spurious variable > assignments > > 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-st > >>>>> andards-sp/content/v/spurious_assign_bz_607 > >>>>> - > >>>>> https://lersek.gitbooks.io/laszlo-s-fork-of-the- > edk-ii-c-coding-st > >>>>> andards- > sp/content/v/spurious_assign_bz_607/6_documenting_softw > are > >>>>> /62_comments.html > >>>>> - > >>>>> https://lersek.gitbooks.io/laszlo-s-fork-of-the- > edk-ii-c-coding-st > >>>>> andards- > sp/content/v/spurious_assign_bz_607/6_documenting_softw > are > >>>>> /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 > <leif.lindh...@linaro.org> > >>>> > >>>> 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 > <leif.lindh...@linaro.org> > >> > >> Best Regards, > >> > >> Leif > >> > >>> Thanks, > >>> Laszlo > >>> > >>>> Regards, > >>>> > >>>> Leif > >>>> > >>>>> Thanks, > >>>>> Laszlo > >>>>> > >>>>> Cc: Andrew Fish <af...@apple.com> > >>>>> Cc: Leif Lindholm <leif.lindh...@linaro.org> > >>>>> Cc: Michael D Kinney <michael.d.kin...@intel.com> > >>>>> Cc: Rebecca Cran <rebe...@bsdio.com> > >>>>> > >>>>> 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 > >>>>> > >> > >> > >> > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47384): https://edk2.groups.io/g/devel/message/47384 Mute This Topic: https://groups.io/mt/33157541/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
