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

["Ryszard Knop" <ryszard.knop@linux.intel.com>]

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 <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.

Richard

> 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
> > > > 
> 
> 
>