Re: [U-Boot-Users] [PATCH 1/11] Fix miscellaneous typos

wd@denx.de wrote on 05/05/2006 03:49:03 PM:

Dear Keith,

I'm a bit unhappy about your patches.

First, I don't know how to make heads or tails out of it.

In this subject you write "PATCH 1/11" which looks as if this were a couple of related patches coming in 11 parts, but actually all of them seem to be completely independent?

Independent, as you requested but with a poor choice of naming on my part. I'll staighten this out.

Then, I received patches 5/11, 8/11, 2/11, 11/11, 6/11, 1/11, 9/11, and 4/11. What about parts 3, 5, 7, 9, and 10?

In message <OFBEDA0037.AED1556F-ON07257165.005DED58-07257165. 005E506A@mck.us.ray.com> you wrote:

...

This patch fixes a few typos. No CHANGELOG entry due to trivial

nature of

...

patch

Where in the README does it say that a CHANGELOG entry is not necessary for simple patches? I need this for each and every patch.

Saw someone else do this on the list a while back. Sorry. I'll put in CHANGELOG entries for all patches.

Then, you are violating the Coding Style requirements in a couple of places (I didn't check carefully as I was not able to apply the patches [see below], but I've seen at least indentation with spaces instead of the required TABs, indentation not by multiples of 8, and trailing white space.

Some "imported code" had this stuff, and in other cases existing code was indented by 4. I'll take a look and try to clean things up.

Then you add nasty editor commands to your sources, like this one: /* vim: set ts=4 tw=80 sw=4 cindent fo=tcroq: */

I'll remove this stuff.

All of this is bad, but this one is fatal and forces me to reject all your patches as they are not usable: your patches are line-wrapped which makes it impossible to apply them; for example, in [PATCH 8/11] Add support for environment storage in SystemACE:

... +- CFG_ENV_IS_IN_SYSACE:

•   Define this if you have a Compact Flash (CF) card connected to a
  

•   Xilinx SystemACE controller in which you wish to store the 
  

environment.

•   - CFG_SYSTEMACE_ENV_DEV:
  

•   The device (instance) number of the SystemACE device you wich to 
  

use.

•   This is normally zero for a system with a single SystemACE.
  

•   - CFG_ENV_SIZE:
  

•   The size of the environment in bytes.  Note that presently this 
  

must ...

Crappy mail client. Is is OK to post all patches as attachements?

Please clean up and resubmit, but please the rules as documetned in the README. And make sure to configure your mailer correctly so that it doesn't corrupt your postings.

All patches rejected. Sorry.

I'll address your issues and resubmit.

Best regards,

Wolfgang Denk

-- Software Engineering: Embedded and Realtime Systems, Embedded Linux Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de In C we had to code our own bugs, in C++ we can inherit them.