> > On Jan 24, 2018, at 9:14 AM, Ben Pfaff <[email protected]> wrote: > > > > On Wed, Jan 24, 2018 at 01:55:18PM +0000, Stokes, Ian wrote: > >>> -----Original Message----- > >>> From: [email protected] [mailto:ovs-dev- > >>> [email protected]] On Behalf Of Justin Pettit > >>> Sent: Wednesday, January 24, 2018 2:31 AM > >>> To: [email protected] > >>> Subject: [ovs-dev] [PATCH] checkpatch.py: Add check for "xxx" in > comments. > >>> > >>> "xxx" is often used to indicate items that the developer wanted to > >>> look at again before committing. Flag those as a warning. > >> > >> Does this mean that code that contains 'xxx' should not be accepted? I > guess ideally we'd want a clean run from the checkpatch script when > submitting/reviewing patches. > > > > I guess that clean "checkpatch" is ideal, but I apply a lot of patches > > that do give checkpatch warnings because checkpatch isn't perfect. I > > think of checkpatch as something that raises possible issues that a > > human should look at. > > 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. Ian > > --Justin > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
