public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Tom Rini <trini@konsulko.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [RESEND PATCH v2 2/4] clk: clk_fixed_rate: Fix style violation
Date: Mon, 15 Jan 2018 08:05:22 -0500	[thread overview]
Message-ID: <20180115130522.GU4660@bill-the-cat> (raw)
In-Reply-To: <CAN1kZoq1A+-TVA69XAPYT71xJcZYPd-Jnv=pQvST7LAbG-4pCA@mail.gmail.com>

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.tomsich@theobroma-systems.com> wrote:
> > Tom,
> >
> >> On 15 Jan 2018, at 11:06, Mario Six <mario.six@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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180115/032768b1/attachment.sig>

  reply	other threads:[~2018-01-15 13:05 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-15 10:06 [U-Boot] [RESEND PATCH v2 1/4] clk: clk-uclass: Fix style violations Mario Six
2018-01-15 10:06 ` [U-Boot] [RESEND PATCH v2 2/4] clk: clk_fixed_rate: Fix style violation Mario Six
2018-01-15 10:19   ` Dr. Philipp Tomsich
2018-01-15 10:44     ` Mario Six
2018-01-15 13:05       ` Tom Rini [this message]
2018-01-22  0:41         ` Simon Glass
2018-01-15 10:06 ` [U-Boot] [RESEND PATCH v2 3/4] clk: Remove superfluous gd declarations Mario Six
2018-01-22  0:41   ` Simon Glass
2018-01-15 10:06 ` [U-Boot] [RESEND PATCH v2 4/4] clk: Makefile: Sort entries alphabetically Mario Six
2018-01-22  0:42   ` Simon Glass
2018-01-22  0:41 ` [U-Boot] [RESEND PATCH v2 1/4] clk: clk-uclass: Fix style violations Simon Glass

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180115130522.GU4660@bill-the-cat \
    --to=trini@konsulko.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox