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

["Leif Lindholm" <leif.lindholm@linaro.org>]

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