From: Joe Perches <joe@perches.com>
To: Avraham Shukron <avraham.shukron@gmail.com>,
Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: jikos@kernel.org, benjamin.tissoires@redhat.com,
linux-usb@vger.kernel.org, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] hid: usbhid: usbkbd: fix checkpatch.pl issues
Date: Wed, 01 Mar 2017 12:46:22 -0800 [thread overview]
Message-ID: <1488401182.20731.11.camel@perches.com> (raw)
In-Reply-To: <3c67c4db-9b59-82f1-9406-3dfdb2ac0a48@gmail.com>
On Wed, 2017-03-01 at 21:59 +0200, Avraham Shukron wrote:
> >
> > This kind of change is definitely not helpful. The original table was
> > Nx16, you converted it to Nx14. Why do you think original table used 16
> > columns?
> >
> > Regardless, it's a very old driver, just let it be.
> >
> > Thanks.
> >
>
> I can make it Nx8 :)
>
> Seriously now - I don't understand what is so wrong with checkpatch fixes?
Some say is makes following logical changes more difficult.
git blame, etc... I'm not a big adherent of that though.
> I'm a new to kernel development, and the natural place to start is to do some
> coding style fixes.
> I thought fixing a driver that I actually use daily will be more satisfying.
> Why driver being old is a good reason to ignore the coding style conventions?
The space after comma rule _should_ be broken when
alignment is more obvious. The existing code is just fine.
Conventions are just that. It's OK to be unconventional.
It's better to know when to follow and when not to follow
those conventions.
checkpatch messages should be considered guides and not
dicta that must be followed to the last letter.
To me, the rest of the patch was OK though.
If you want to learn the kernel patch process, modifying files
in drivers/staging is a good place to start.
Otherwise, it's probably best to find/fix some actual defect
and suggest actual logical changes that reduce code size,
improve performance, expand test coverage, better document
the code, etc..
cheers, Joe
next prev parent reply other threads:[~2017-03-01 20:46 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-21 17:26 [PATCH] hid: usbhid: usbkbd: fix checkpatch.pl issues Avraham Shukron
2017-03-01 18:16 ` Dmitry Torokhov
2017-03-01 19:59 ` Avraham Shukron
2017-03-01 20:43 ` Dmitry Torokhov
2017-03-01 20:46 ` Joe Perches [this message]
[not found] ` <1488401182.20731.11.camel-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
2017-03-01 20:55 ` Avraham Shukron
2017-03-01 20:56 ` Greg KH
2017-03-01 22:05 ` Alan Stern
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1488401182.20731.11.camel@perches.com \
--to=joe@perches.com \
--cc=avraham.shukron@gmail.com \
--cc=benjamin.tissoires@redhat.com \
--cc=dmitry.torokhov@gmail.com \
--cc=jikos@kernel.org \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).