Prev: MMC:Add support MMCIF for SuperH
Next: linux-next: manual merge of the staging-next tree with the pcmcia tree
From: Greg KH on 28 Apr 2010 01:40
On Tue, Apr 27, 2010 at 10:30:03PM -0700, Joe Perches wrote:
> On Tue, 2010-04-27 at 16:02 -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.
>
> You should be more consistent.

I am human.

> http://www.tuxradar.com/content/newbies-guide-hacking-linux-kernel
> Greg even said, "submit one big patch if you want"

Yes, but I will then help to guide them to make it a more reasonable,
and reviewable, patch :)

> > It also doesn't apply due to
> > other changes in these files (that you then remove in patch 3) from
> > other developers.
>
> No, it doesn't apply because you chose to apply other patches
> before this one.

Those patches were sent to me before yours, sorry.

thanks,

greg "slowly catching up" k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Greg KH on 28 Apr 2010 02:00
On Tue, Apr 27, 2010 at 10:44:34PM -0700, Joe Perches wrote:
> On Tue, 2010-04-27 at 22:34 -0700, Greg KH wrote:
> > > No, it doesn't apply because you chose to apply other patches
> > > before this one.
> > Those patches were sent to me before yours, sorry.
>
> By like an hour. They were ugly so I cleaned them.

No, one was 14 days before yours, the other, 2 1/2 hours.

> And besides, you didn't act on them for over a month.

That has already been discussed.

> I seem to be learning that you little taste.

-ENOPARSE
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Dan Carpenter on 28 Apr 2010 08:20
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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: MMC:Add support MMCIF for SuperH
Next: linux-next: manual merge of the staging-next tree with the pcmcia tree