Re: [edk2-devel] [PATCH edk2-CCSS 0/3] Coding Standards: add rule for documenting spurious variable assignments

["Michael D Kinney" <michael.d.kinney@intel.com>]

Series Reviewed-by: Michael D Kinney <michael.d.kinney@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 <lersek@redhat.com>
> Sent: Wednesday, September 11, 2019 10:51 AM
> To: devel@edk2.groups.io; ryszard.knop@linux.intel.com;
> leif.lindholm@linaro.org
> Cc: Andrew Fish <afish@apple.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Rebecca Cran
> <rebecca@bsdio.com>; Philippe Mathieu-Daude
> <philmd@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.lindholm@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.lindholm@linaro.org>
> >>
> >> Best Regards,
> >>
> >> Leif
> >>
> >>> Thanks,
> >>> Laszlo
> >>>
> >>>> Regards,
> >>>>
> >>>> Leif
> >>>>
> >>>>> Thanks,
> >>>>> Laszlo
> >>>>>
> >>>>> Cc: Andrew Fish <afish@apple.com>
> >>>>> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> >>>>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> >>>>> Cc: Rebecca Cran <rebecca@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
> >>>>>
> >>
> >>
> >>
> >
> >
> > 
> >