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

["Laszlo Ersek" <lersek@redhat.com>]

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