Re: How do we review changes made with coccinelle?

[Junio C Hamano <gitster@pobox.com>]

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.

> - 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.  Your use of memcpy() may be rewritten to COPY_ARRAY()
when appropriate.  At least that is the theory---an overly wide or
ad-hoc rule that depends too much on heuristic may misconvert future
code, which needs to be caught by reviewers when .cocci files are
added to the tree.

> - Do we care about new patches slowing down coccicheck?

Surely.

We may want to cull rules from time to time.  For example, a rule
that moves callers of an older API function to use a newer API
function has to be kept while the older API function still exists in
the tree to help topics that are still in flight, but eventually
everybody stops using it and the implementation of the older API
function gets removed.  We should make sure we remove the rule that
is now stale.