From mboxrd@z Thu Jan 1 00:00:00 1970 From: Detlev Zundel Date: Wed, 20 Apr 2011 15:27:26 +0200 Subject: [U-Boot] checkpatch - theory and practice [was: [PATCH v2 3/6] TFTP: rename "server" to "remote"] In-Reply-To: <4DAEBB90.7070307@comelit.it> (Luca Ceresoli's message of "Wed, 20 Apr 2011 12:55:12 +0200") References: <1302796377-3321-1-git-send-email-luca.ceresoli@comelit.it> <1303143594-5345-4-git-send-email-luca.ceresoli@comelit.it> <4DADAA50.1040008@comelit.it> <4DAEBB90.7070307@comelit.it> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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