Re: [PATCH 2/8] staging: hv: Changed |typedef RING_BUFFER| to |struct hv_ring_buffer| throughout.

[Arnd Bergmann]

On Tuesday 22 September 2009, Steve Friedl wrote:
> The style guide (and Greg's prior work) have suggested that the typedef
> UPPERCASENAME style is not appropriate in the Linux kernel (even though
> it's been de rigueur in Win32 development for 20 years), so I'm fixing
> a couple more of them that remain... one at a time.
> 
> ~~~ Steve
> 
> Signed-off-by: Steve Friedl <steve@xxxxxxxxxxx>

A few comments about your submission, not so much about this changes
in this patch specifically.

First of all, welcome here and thanks for your first submission!
Content-wise, all your patches look correct so far, nice work.

The patch comment above mixes both information about the contents of the
patch and information about the submission. The latter (as well as your
email signature) have no place in the changeset comment above the
'---' line but should go below it. They are good to have in the mail
but serve no purpose in the git history afterwards. As an example, you
could have written the same introduction as:

|[PATCH 2/8] staging: hv: Change typedef RING_BUFFER to struct hv_ring_buffer
|
|The typedef UPPERCASENAME style is not appropriate in the Linux kernel
|(even though it's been de rigueur in Win32 development for 20 years),
|so change 'typedef RING_BUFFER' to 'struct hv_ring_buffer'.
|
|Signed-off-by: Steve Friedl <steve@xxxxxxxxxxx>
|
|---
| The style guide (and Greg's prior work) have suggested doing that,
| and I'm fixing a couple more of them that remain... one at a time.
|
| ~~~ Steve
|
| drivers/staging/hv/RingBuffer.c |    6 +++---
| drivers/staging/hv/RingBuffer.h |    6 +++---
| 2 files changed, 6 insertions(+), 6 deletions(-)

It's also preferred to have the full series in a single email thread,
with all patches as a reply to the introductory mail. git-send-email
does that with the '--thread --no-chain-reply-to', git-format-patch
does the same with 'thread=shallow' (great job on consistency ;-) ).

You should also Cc the right mailing lists and people for the subsystem
you are patching. If you don't know them, use scripts/get_maintainer.pl.
'scripts/get_maintainer.pl -f drivers/staging/hv/*' currently returns
Greg Kroah-Hartman <gregkh@xxxxxxx>
Nicolas Palix <npalix@xxxxxxx>
Hank Janssen <hjanssen@xxxxxxxxxxxxx>
Haiyang Zhang <haiyangz@xxxxxxxxxxxxx>
Bill Pemberton <wfp5p@xxxxxxxxxxxx>
Randy Dunlap <randy.dunlap@xxxxxxxxxx>
Jan Beulich <jbeulich@xxxxxxxxxx>
Moritz Muehlenhoff <jmm@xxxxxxxxxx>
devel@xxxxxxxxxxxxxxxxxxxx
linux-kernel@xxxxxxxxxxxxxxx

By looking at 'git log drivers/staging/hv', you can also determine that
Randy, Jan and Moritz each only contributed a single patch, so they
are not likely to be interested in your changes, but it would not
be harmful to Cc them.

Also, you should check if there is a maintainer tree that already addresses
the problems you are fixing or if there are possibly conflicts with other
changes in that tree, not sure if you did that but I've seen a number of
similar patches for the hv drivers so it would be possible that someone
else did the same already.

	Arnd <><
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/