lkml.org 
[lkml]   [2017]   [Dec]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v4 72/73] xfs: Convert mru cache to XArray
    On Sat, Dec 09, 2017 at 09:00:18AM -0800, Joe Perches wrote:
    > On Sat, 2017-12-09 at 09:36 +1100, Dave Chinner wrote:
    > > 1. Using lockdep_set_novalidate_class() for anything other
    > > than device->mutex will throw checkpatch warnings. Nice. (*)
    > []
    > > (*) checkpatch.pl is considered mostly harmful round here, too,
    > > but that's another rant....
    >
    > How so?

    Short story is that it barfs all over the slightly non-standard
    coding style used in XFS. It generates enough noise on incidental
    things we aren't important that it complicates simple things. e.g. I
    just moved a block of defines from one header to another, and
    checkpatch throws about 10 warnings on that because of whitespace.
    I'm just moving code - I don't want to change it and it doesn't need
    to be modified because it is neat and easy to read and is obviously
    correct. A bunch of prototypes I added another parameter to gets
    warnings because it uses "unsigned" for an existing parameter that
    I'm not changing. And so on.

    This sort of stuff is just lowest-common-denominator noise - great
    for new code and/or inexperienced developers, but not for working
    with large bodies of existing code with slightly non-standard
    conventions. Churning *lots* of code we otherwise wouldn't touch
    just to shut up checkpatch warnings takes time, risks regressions
    and makes it harder to trace the history of the code when we are
    trying to track down bugs.

    Cheers,

    Dave.
    --
    Dave Chinner
    david@fromorbit.com

    \
     
     \ /
      Last update: 2017-12-11 22:43    [W:5.491 / U:0.088 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site