Re: [PATCH 0/6] net: ceph: messenger: code cleanup

From: Ilya Dryomov <idryomov@gmail.com>
To: Ioana Ciornei <ciorneiioana@gmail.com>
Cc: Ceph Development <ceph-devel@vger.kernel.org>,
	Alex Elder <elder@ieee.org>
Subject: Re: [PATCH 0/6] net: ceph: messenger: code cleanup
Date: Wed, 27 Jan 2016 12:17:18 +0100	[thread overview]
Message-ID: <CAOi1vP-Lqj4iA+cjFY1D3ERMdCyFxwkXTGEPpY2_oNJ9D-4PXA@mail.gmail.com> (raw)
In-Reply-To: <cover.1453853617.git.ciorneiioana@gmail.com>

On Wed, Jan 27, 2016 at 1:43 AM, Ioana Ciornei <ciorneiioana@gmail.com> wrote:
>
> This patchset makes some small code cleanup on the messenger code from net/ceph.
> Most of the patches are a result of running checkpatch.pl on
> net/ceph/messenger.c
>
>
> Ioana Ciornei (6):
>   net: ceph: messenger: remove unnecessary blank spaces
>   net: ceph: messenger: fix coding style comparison to NULL
>   net: ceph: messenger: add blank line after declarations
>   net: ceph: messenger: properly align function parameters
>   net: ceph: messenger: change printk to pr_warn
>   net: ceph: messenger: add braces to all arms of conditional statement
>
>  net/ceph/messenger.c | 207 ++++++++++++++++++++++++++-------------------------
>  1 file changed, 107 insertions(+), 100 deletions(-)

Hi Ioana,

Most of above (especially patch 4/6) just clutters the history (think
"git blame" output) and, more importantly, complicates backporting of
future fixes to older kernels for no benefit at all.

There is a reason checkpatch.pl is called "check" "patch" - it is to be
run on patches, not files, with pretty much the only exception to that
being code in drivers/staging.  In certain cases, it might be OK to use
it on existing files to fix what it classifies as errors, but fixing
warnings or so called checks does more harm than good in most cases.

Thanks,

                Ilya