On Mon, Jan 15, 2018 at 11:44:46AM +0100, Mario Six wrote: > On Mon, Jan 15, 2018 at 11:19 AM, Dr. Philipp Tomsich > <philipp.toms...@theobroma-systems.com> wrote: > > Tom, > > > >> On 15 Jan 2018, at 11:06, Mario Six <mario....@gdsys.cc> wrote: > >> > >> Fix a mis-indented function call in clk_fixed_rate.c > > > > A general question: do we want to have such gardening commits > > create an additional indirection in our history for people using > > git-blame frequently (e.g. I usually use git-blame to find the last > > commit that touched a line and then read the log message to find > > out why something was changed… now I’d have to restart this > > search whenever I hit a pure formatting change)?
It does make archaeology harder at times, true. > > My gut feeling would be that we should try to change lines only > > when there is an actual change to the code happening. > > > > From https://www.denx.de/wiki/U-Boot/Patches: > > "Non-functional changes, i.e. whitespace and reformatting changes, should be > done in separate patches marked as cosmetic. This separation of functional and > cosmetic changes greatly facilitates the review process." > > (granted, I didn't explicitly mark the patches as cosmetic) > > I read that as a general permission to post style-fix patches. If there's a > different consensus, I'd like the page modified accordingly. But this is also true (and yes, so long as the commit is otherwise clear that it's coding style, etc, fixes, it's not also marked as cosmetic) that we do in fact want these clean-ups. The biggest problem I see is that checkpatch.pl isn't as easily integrated into our workflow as other CI tools. So new problems get in. Now, it's not as hard as it might have been ages ago, and I can drop checkpatch.pl -q --git origin/master.. into my build scripts and get something not too bad to review to try and catch pretty bad formatting problems at least. -- Tom
signature.asc
Description: PGP signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
