[lm-sensors] [PATCH] drivers/hwmon: fix trailing whitespace
Frans Meulenbroeks
fransmeulenbroeks at gmail.com
Fri Jan 6 09:35:05 CET 2012
2012/1/6 Jean Delvare <khali at linux-fr.org>
> Hi Frans,
>
> [...]
> > > not have any checkpatch errors or warnings.
> > (...)
> > Probably i should explain a little bit on my way of working.
> > For me the most efficient way to tackle this is to only fix one error
> type
> > at a time.
> > This reduces the chance of mistakes (like mixing up simple_strtoul and
> > strict_strtoul), and also makes doing the edits simpler.
>
> That's a bad example IMHO. These two changes should really go in the
> same commit, as they are so similar. You can easily generate them
> separately and then merge them together before sending.
>
> > However the side effect of this is that not all problems in a file are
> > solved.
>
> I definitely prefer doing it the other way around: take one driver and
> fix all (or most) style issues. While it is slightly more time
> consuming to review, it has advantages:
> * When backporting a driver to an older kernel, you can pick all
> patches touching the driver, without having to filter out all changes
> affecting other drivers.
> * Checkpatch is happy on every patch, which compensates for the harder
> review.
> * Per-driver patches are easier for me and Guenter to handle. We tend
> to each concentrate on a subset of the drivers for each release
> cycle, so that our trees don't collide in linux-next. With
> cross-driver patches we can't do that, which basically puts all the
> handling burden on a single one of us for the whole release cycle.
> * It makes the history of each individual driver much easier to read.
> Having 10 patches in the history to clean up 10 different style
> issues doesn't exactly help.
>
> > PS: note that I don't own most of the hardware, so i cannot really
> run-time
> > verify my work, only do a compile-test.
>
> And so:
> * Testing by people with the hardware is much easier.
>
> If you still prefer cross-driver patches, I guess that's better than
> not getting contributions from you at all, but then please at least
> ensure that checkpatch doesn't complain about your patches, even if
> that means fixing more than one issue at once. BTW, all whitespace
> fixes can go in the same patch to start with, it makes sense to group
> them IMHO.
>
> Hi Jean,
Thanks for your feedback.
I did not really consider things from the perspective of the reviewer and
user.
Please ignore this patch. I'll revert back to file specific patches.
Note that I am not planning to clean all files with checkpatch problems,
but I'll probably do a few more inbetween other things (e.g. waiting for a
kernel rebuild).
Best regards, Frans
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20120106/775e5d45/attachment.html>
More information about the lm-sensors
mailing list