From: Dan Carpenter <error27@gmail.com>
To: Greg KH <greg@kroah.com>
Cc: Joe Perches <joe@perches.com>,
devel@driverdev.osuosl.org, Scott Smedley <ss@aao.gov.au>,
Greg KH <gregkh@suse.de>, Jason Baldus <jason.baldus@gmail.com>,
linux-kernel@vger.kernel.org,
Harley Sweeten <hsweeten@visionengravers.com>,
Simon Horman <horms@verge.net.au>
Subject: Re: [PATCH 1/3] drivers/staging/dt3155: Integrate 3 badly styled files into 1 clean file
Date: Wed, 28 Apr 2010 14:18:52 +0200 [thread overview]
Message-ID: <20100428121851.GB24513@bicker> (raw)
In-Reply-To: <20100427230228.GB17260@kroah.com>
On Tue, Apr 27, 2010 at 04:02:28PM -0700, Greg KH wrote:
> On Mon, Mar 29, 2010 at 12:34:28AM -0700, Joe Perches wrote:
> > Make variables static where appropriate
> > Rename dt3155_<foo_with_long_names> variables to dt_<foo_tla>
> > to reduce code length and make more lines fit well in 80 chars
> > Remove now unnecessary .h files
> > Change indent to use tabs
> > Remove unused functions
> > Used bool more often
> > Checkpatch clean
>
> This is too much in one patch, sorry. It also doesn't apply due to
> other changes in these files (that you then remove in patch 3) from
> other developers.
>
What I like is when people send in a patch that addresses all the white
space issues at once. So from the above list it would probably fix these:
> > to reduce code length and make more lines fit well in 80 chars
> > Change indent to use tabs
> > Checkpatch clean
I have a script to check that a patch only changes whitespace and I can
check that directly from mutt. It's in the my smatch repo:
git://repo.or.cz/smatch.git under smatch_scripts/whitespace_only.sh
If that passes and the source still compiles then I'm pretty sure it
doesn't break anything. At that point I can look through it focusing on
the whitespace instead of worrying that the patch introduces bugs.
Once the whitespace is fixed the rest of the changes can be sent in
separately and they're much smaller and easier to audit.
regards,
dan carpenter
next prev parent reply other threads:[~2010-04-28 12:19 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-03-28 15:26 Staging: dt3155: fixed some checkpatch.pl warnings and errors in dt3155_drv.c Olimpiu Pascariu
2010-03-29 3:23 ` Simon Horman
2010-03-29 7:34 ` [PATCH 0/3] drivers/staging/dt3155: Large cleanups Joe Perches
2010-03-29 7:34 ` [PATCH 1/3] drivers/staging/dt3155: Integrate 3 badly styled files into 1 clean file Joe Perches
2010-04-27 23:02 ` Greg KH
2010-04-28 5:30 ` Joe Perches
2010-04-28 5:34 ` Greg KH
2010-04-28 5:44 ` Joe Perches
2010-04-28 5:52 ` Greg KH
2010-04-28 6:00 ` Joe Perches
2010-04-28 16:12 ` Valdis.Kletnieks
2010-04-28 16:33 ` Joe Perches
2010-04-28 17:22 ` Greg KH
2010-04-28 12:18 ` Dan Carpenter [this message]
2010-03-29 7:34 ` [PATCH 2/3] drivers/staging/dt3155/allocator: sparse cleanups Joe Perches
2010-03-29 7:34 ` [PATCH 3/3] drivers/staging/dt3155: Remove old files Joe Perches
2010-03-29 17:35 ` Staging: dt3155: fixed some checkpatch.pl warnings and errors in dt3155_drv.c Olimpiu Pascariu
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=20100428121851.GB24513@bicker \
--to=error27@gmail.com \
--cc=devel@driverdev.osuosl.org \
--cc=greg@kroah.com \
--cc=gregkh@suse.de \
--cc=horms@verge.net.au \
--cc=hsweeten@visionengravers.com \
--cc=jason.baldus@gmail.com \
--cc=joe@perches.com \
--cc=linux-kernel@vger.kernel.org \
--cc=ss@aao.gov.au \
/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