From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wolfgang Denk Date: Mon, 21 Mar 2011 22:34:00 +0100 Subject: [U-Boot] [PATCH v6 4/4] mpc85xx: Add board support for the eXMeritus HWW-1U-1A devices In-Reply-To: References: <1300208664-18339-1-git-send-email-Kyle.D.Moffett@boeing.com> <1300208664-18339-5-git-send-email-Kyle.D.Moffett@boeing.com> <20110315193650.DFB971518DB9@gemini.denx.de> <5B9D9C87-C278-4AF3-B20C-26ECFF6C09B7@boeing.com> <20110321203007.76192DEE10D@gemini.denx.de> Message-ID: <20110321213401.005E4CF@gemini.denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Dear "Moffett, Kyle D", In message you wrote: > > Just looking at the last ~200 commits (actually 187, because it ignores mer= > ges): > > $ git format-patch -o recent-patches -200 origin/master > $ ./checkpatch.pl --no-tree --strict recent-patches/* >checkpatch.log 2>&1 > $ grep 'over 80 char' checkpatch.log | wc -l > 130 > > That's 130 lines in the last 200 patches which are over 80 characters?!?! > How are those patches any different from mine? The difference is: They were not detected. Patches welcome. > Look, I'm really trying to comply with U-Boot coding standards, but I'm rea= > lly > of pissed off about the inconsistent requirements you are applying to my > patches versus a lot of other things that YOU ARE MERGING on a regular basi= > s. The requirements are NOT inconsistent. It's just that nobody is perfect, and nobody ever claimed that we manage to get 100% of review coverage. > So why are you picking on my board-specific code so hard here? This is It's just that the problems got noticed. > > "checkpatch.pl --no-tree" > > What version of checkpatch are you running? I copied version 0.31 out of > my latest Linux kernel tree, which identical to the latest version from > here: -> checkpatch.pl --version Usage: checkpatch.pl [OPTION]... [FILE]... Version: 0.31 ... > If U-Boot policy is to run checkpatch then you'd better either specify a > particular version and command-line options or be willing to accept the > default output of the latest version. I don't see any version issue here, nor do I use any non-standard options. > >>>> +U_BOOT_CMD( > >>>> + hww1u1a_test_cpu_a, 1, 0, do_hww1u1a_test_cpu_a, > >>>> + "Test if this is CPU A (versus B) on the eXMeritus HWW-1U-1A board", > >>>> + /* */" && \n" > >>>> + "hww1u1a_test_cpu_a || \n" > >>> =20 > >>> What is this empty comment needed for? > >> > >> Just a mental placeholder for the fact that the U_BOOT_CMD macro inserts > >> the name of the command in that spot. Will remove. > > > > We don't provide usage examples in the help text. This should be > > fixed in the first place. > > This *IS* the help text, and not a sample usage. This is visually and > effectively no different from this text in common/cmd_mp.c: The synopsis of a command gives the command name and possible options, and explains what the command does and what the options do. "name && " does NOT fall into that pattern. Look, alternatively I can claim your help message is highly incomplete as it fails to cover use cases like name || etc. Not to mention that the usage message is plain wrong for all boards that don't have the hush shell enabled. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Ninety-Ninety Rule of Project Schedules: The first ninety percent of the task takes ninety percent of the time, and the last ten percent takes the other ninety percent.