On 6/20/22 2:18 AM, Theodore Ts'o wrote:
On Sun, Jun 19, 2022 at 11:21:27AM +0800, Liu Peibao wrote:
Thanks for your reply. What I want do to is rename some temporary variables
in the patch2 and when I make the patch, there are the checkpatch warnings.
From the point of view "one patch do one thing", I split the modification
into two patches. Thanks!
I didn't really see the poiont of renaming the temporary variables,
either.
In this particular case basically only used to avoid line lengths from
exceeding ~72 characters, and requiring a line wrap, and bio_start and
bio_end is used only in one place in the code block below.
Is it _really_ all that confusing whether they are named
bio_{start,end} instead of bvec_{start,end}?
If I was writing that code from scratch, I might have just used start
and end without any prefixes. And as far as "only have a patch do one
thing at a time", this doesn't apply to checkpatch fixes.
The basic motivation behind "no checkpatch-only fixes" is that it
tends to introduce code churn which makes interpreting information
from "git blame" more difficult; and so therefore the costs exceed the
extremely marginal benefits of fixing most checkpatch complaints. So
making a _patch_ be checkpatch clean, whether it's modifying existing
code or writing new code, is fine, since you're making a subtantive
change to the code, so this is as good a time as any to fix up tiny
nits such as checkpatch complaints.
But the idea behind "no unnecessary code churn since it ruins git
blame and could potentially induce future patch conflicts" also
applies to renaming variables. The benefits are very minor, and they
don't outweigh the costs.
- Ted
Got it! Thanks for your detailed and comprehensive explanation!
Best Regards,
Peibao
