2012/1/6 Jean Delvare <khali@xxxxxxxxxxxx>
Hi Frans,
[...]
> > not have any checkpatch errors or warnings.> (...)
> Probably i should explain a little bit on my way of working.That's a bad example IMHO. These two changes should really go in the
> 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.
same commit, as they are so similar. You can easily generate them
separately and then merge them together before sending.
I definitely prefer doing it the other way around: take one driver and
> However the side effect of this is that not all problems in a file are
> solved.
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.
And so:
> 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.
* 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
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
_______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
