Re: How do we review changes made with coccinelle?

From: Glen Choo <chooglen@google.com>
Date: Mar 31, 2023, 17:17 UTC
Message-ID: <kl6lzg7sc01z.fsf@chooglen-macbookpro.roam.corp.google.com>
In-Reply-To: <xmqqtty2hx30.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

Show 9 quoted lines

> Glen Choo <chooglen@google.com> writes:
>
>> - Is it okay to give Reviewed-By on the basis of _just_ the in-tree
>>   changes and ignore the .cocci patch?
>
> If they were made in separate steps, sure.  If not, not really.  But
> we can still say "I've checked the changes the author made to the
> code and they looked good."  But we haven't reviewed the patch in
> its entirety in such a case to give a Reviewed-by, I would thihk.

I agree, but I wonder what this means in practice when .cocci fluency is so low. Maybe:

- We increase .cocci fluency, partly by creating our own learning
  materials, partly by being more conscious about spreading knowledge
  from the folks who are relatively fluent in it. We might not have to
  do a lot to get the ball rolling either; just checking in a
  "MyFirstCocci" would help a lot, I think.

- Define some guidelines for what it means for a ".cocci" to be 'good
  enough'. E.g.:

  - Do rules need to target just the right things or is some collateral
    damage okay? Does this change between .cocci and .pending.cocci?

  - Do we allow copy-pasting in .cocci rules when a more 'elegant'
    alternative exists?

Show 5 quoted lines

>> - What do we do with .cocci after they've been applied?
>
> When we keep .cocci rules in tree, "make coccicheck" would complain
> on any new code that matches the preimage pattern of these rules and
> adjust them.

Orthogonal to the in tree .cocci rules that enforce code style, there are .cocci meant for large scale refactoring.

IIUC at least one other opinion there is that we keep such refactoring rules in tree temporarily for the benefit of in-flight or out-of-tree conflicts [1], but since they are defunct by definition, we remove them eventually.

[1] https://lore.kernel.org/git/230326.86ileow1fu.gmgdl@evledraar.gmail.com/