Justin Pettit <jpet...@ovn.org> writes:

>> On Jan 25, 2018, at 10:26 AM, Stokes, Ian <ian.sto...@intel.com> wrote:
>> 
>>> Yes, this was my thinking too.  I don't think we should have a prohibition
>>> against using "xxx", but I do usually see it in patches as something that
>>> was left behind unintentionally.  That said, my personal preference would
>>> be to not have that often in the codebase, since it usually indicates
>>> something half-baked, and more often than not, seems to be something no
>>> one comes back to address later on.
>> 
>> I'd agree, I guess there should be a valid explanation as to why it
>> hasn't been implemented in the associated XXX comment. It'll
>> definitely help to raise it as a point of discussion during the
>> review. Sounds good to me.
>
> Great.  Thanks!

Because more voices of support are better - I am in favor of this
change.

I agree with the sentiment that checkpatch is meant to help bring things
to light that humans sometimes glaze over (we all develop our own way of
'reading' code - for me I just don't *see* whitespace anymore, so
checkpatch helps me self enforce it).  I don't think I'd ever trust a
machine, even one as snazzy as checkpatch, with true veto power over a
change.  In that vein, this helps draw the eye to "hey there's really
something incomplete here - perhaps it is worth taking a closer look."

Especially since I have been known to leave things behind - and having
the checkpatch hook catch it for me early is better.

All that long winded response to simply:

Acked-by: Aaron Conole <acon...@redhat.com>


On a side note - I was really happy to see that the in-comment tracking
worked for most of the cases I tried.  The only one that didn't catch
is:

  /*
   XXX : something here
   */

but I don't think it's worth trying to solve that :)

> --Justin
>
>
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to