All of lore.kernel.org
 help / color / mirror / Atom feed
From: Detlev Zundel <dzu@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] checkpatch - theory and practice [was: [PATCH v2 3/6] TFTP: rename "server" to "remote"]
Date: Wed, 20 Apr 2011 15:27:26 +0200	[thread overview]
Message-ID: <m28vv5giip.fsf_-_@ohwell.denx.de> (raw)
In-Reply-To: <4DAEBB90.7070307@comelit.it> (Luca Ceresoli's message of "Wed, 20 Apr 2011 12:55:12 +0200")

Hi Luca,

>>> It's needed for checkpatch compliance.
>> I'm trying to understand the problems involved, but looking at this
>> again, it is not clear to me what you say here.  When I run your version
>> 1 of the patches (where you only do the rename) through checkpatch, I
>> get:
>>
>>    WARNING: line over 80 characters
>>    #116: FILE: net/tftp.c:59:
>>    +static int	TftpRemotePort;		/* The UDP port at their end		*/
>>
>>    WARNING: consider using kstrto* in preference to simple_strtol
>>    #215: FILE: net/tftp.c:619:
>>    +		TftpRemotePort = simple_strtol(ep, NULL, 10);
>>
>>    total: 0 errors, 2 warnings, 99 lines checked
>>
>>    /home/dzu/transfer/p2 has style problems, please review.  If any of these errors
>>    are false positives report them to the maintainer, see
>>    CHECKPATCH in MAINTAINERS.
>>
>> So I'm not sure why you say that the other changes are needed for
>> checkpatch.  What exactly do you mean by this?
>
> All the comments were nicely columned before my patchset. Reducing the
> length of a line would have broken this.
>
> I chose to change all of them in order to preserve the pre-existing
> coding style.

Ok, this makes sense, alas the wording "it's needed for checkpatch
compliance" was somewhat misleading.

Ideally only the relevant changes should be in one commit and
re-indentation to align everything again should be in a separate commit.
As we saw that checkpatch also looks at context lines, this commit
usually needs to be logically _before_ your own changes.  Probably the
easiest way to achieve this is to commit the changes separately and
reorder them with git rebase -i.

I amended the wiki page[1] in the hope of getting more light into these
things.

Cheers
  Detlev

[1] http://www.denx.de/wiki/U-Boot/Patches

-- 
Irrationality is the square root of all evil.
                                     -- Douglas Hofstadter
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

  reply	other threads:[~2011-04-20 13:27 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-13  9:08 [U-Boot] [RFC] Act as a TFTP server Luca Ceresoli
2011-04-13 11:37 ` Wolfgang Denk
2011-04-13 12:47   ` Luca Ceresoli
2011-04-30 19:53     ` Wolfgang Denk
2011-05-03 12:23       ` Luca Ceresoli
2011-05-03 12:36         ` Wolfgang Denk
2011-05-16 20:57           ` Luca Ceresoli
2011-04-14 10:31 ` Luca Ceresoli
2011-04-14 15:52 ` [U-Boot] [PATCH 0/6] " Luca Ceresoli
2011-04-14 16:13   ` Albert ARIBAUD
2011-04-18 16:19   ` [U-Boot] [PATCH v2 " Luca Ceresoli
2011-04-19 14:05     ` Detlev Zundel
2011-05-17 10:03     ` [U-Boot] [PATCH v3 0/5] " Luca Ceresoli
2011-05-17 10:03     ` [U-Boot] [PATCH v3 1/5] TFTP: replace "server" with "remote" in local variable names Luca Ceresoli
2011-05-19  8:11       ` Detlev Zundel
2011-05-19 19:39       ` Wolfgang Denk
2011-05-17 10:03     ` [U-Boot] [PATCH v3 2/5] TFTP: rename STATE_RRQ to STATE_SEND_RRQ Luca Ceresoli
2011-05-19  8:12       ` Detlev Zundel
2011-05-19 19:39       ` Wolfgang Denk
2011-05-17 10:03     ` [U-Boot] [PATCH v3 3/5] TFTP: net/tftp.c: add server mode receive Luca Ceresoli
2011-05-19  8:13       ` Detlev Zundel
2011-05-19 19:48       ` Wolfgang Denk
2011-05-17 10:03     ` [U-Boot] [PATCH v3 4/5] TFTP: add tftpsrv command Luca Ceresoli
2011-05-19  8:12       ` Detlev Zundel
2011-05-19 19:48       ` Wolfgang Denk
2011-05-17 10:03     ` [U-Boot] [PATCH v3 5/5] net/tftp.c: fix typo Luca Ceresoli
2011-05-19  8:13       ` Detlev Zundel
2011-05-19 19:48       ` Wolfgang Denk
2011-04-18 16:19   ` [U-Boot] [PATCH v2 1/6] README: remove spurious line Luca Ceresoli
2011-04-19 14:07     ` Detlev Zundel
2011-04-18 16:19   ` [U-Boot] [PATCH v2 2/6] NET: pass source IP address to packet handlers Luca Ceresoli
2011-04-19 14:15     ` Detlev Zundel
2011-04-19 15:26       ` Luca Ceresoli
2011-05-12 17:38     ` Wolfgang Denk
2011-04-18 16:19   ` [U-Boot] [PATCH v2 3/6] TFTP: rename "server" to "remote" Luca Ceresoli
2011-04-19 14:18     ` Detlev Zundel
2011-04-19 15:29       ` Luca Ceresoli
2011-04-19 16:28         ` Detlev Zundel
2011-04-19 21:56           ` Luca Ceresoli
2011-04-20  8:17             ` Detlev Zundel
2011-05-04  7:58               ` Luca Ceresoli
2011-05-04 10:37                 ` Detlev Zundel
2011-04-20  8:41         ` Detlev Zundel
2011-04-20 10:55           ` Luca Ceresoli
2011-04-20 13:27             ` Detlev Zundel [this message]
2011-05-16 20:35         ` Luca Ceresoli
2011-05-17  9:38           ` Detlev Zundel
2011-04-30  4:36     ` Mike Frysinger
2011-05-16 20:16       ` Luca Ceresoli
2011-05-12 17:46     ` Wolfgang Denk
2011-05-13  7:36       ` Luca Ceresoli
2011-05-14 16:07         ` Luca Ceresoli
2011-04-18 16:19   ` [U-Boot] [PATCH v2 4/6] TFTP: rename STATE_RRQ to STATE_SEND_RRQ Luca Ceresoli
2011-04-19 14:22     ` Detlev Zundel
2011-04-18 16:19   ` [U-Boot] [PATCH v2 5/6] TFTP: net/tftp.c: add server mode receive Luca Ceresoli
2011-04-19 14:37     ` Detlev Zundel
2011-05-17  8:06       ` Luca Ceresoli
2011-05-17  9:41         ` Detlev Zundel
2011-04-18 16:19   ` [U-Boot] [PATCH v2 6/6] TFTP: add tftpsrv command Luca Ceresoli
2011-04-19 14:39     ` Detlev Zundel
2011-04-30  4:32     ` Mike Frysinger
2011-05-04  7:02     ` Mike Frysinger
2011-04-14 15:52 ` [U-Boot] [PATCH 1/6] README: remove spurious line Luca Ceresoli
2011-04-30 20:19   ` Wolfgang Denk
2011-04-14 15:52 ` [U-Boot] [PATCH 2/6] NET: pass source IP address to packet handlers Luca Ceresoli
2011-04-30 20:22   ` Wolfgang Denk
2011-04-30 20:24   ` Wolfgang Denk
2011-05-03  8:54     ` Luca Ceresoli
2011-04-14 15:52 ` [U-Boot] [PATCH 3/6] TFTP: rename "server" to "remote" Luca Ceresoli
2011-04-14 15:52 ` [U-Boot] [PATCH 4/6] TFTP: rename STATE_RRQ to STATE_SEND_RRQ Luca Ceresoli
2011-04-14 15:52 ` [U-Boot] [PATCH 5/6] TFTP: net/tftp.c: add server mode receive Luca Ceresoli
2011-04-14 15:52 ` [U-Boot] [PATCH 6/6] TFTP: add tftpsrv command Luca Ceresoli

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=m28vv5giip.fsf_-_@ohwell.denx.de \
    --to=dzu@denx.de \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.