Re: [PULL] http://linuxtv.org/hg/~stoth/s2

[Steven Toth <stoth@xxxxxxxxxxx>]

Mauro Carvalho Chehab wrote:
> On Sun, 12 Oct 2008 12:14:23 -0400
> Steven Toth <stoth@xxxxxxxxxxx> wrote:
> 
>>> +		printk("%s: tvp.cmd = 0x%08x (undefined/unknown/invalid)\n",
>> I'm slowly going throw all of the files for checpatch, it will catch 
>> this when I get to dvb_frontend.c, dvb_frontend.h and frontend.h
>>
>> Expect a lot of checkpatch updates a few days from now. They are 
>> currently in the stoth/s2-mfe tree, but the checkpatch specifics will be 
>> split out into stoth/checkpatch and be the third tree I ask you to pull.
>>
>> If you can, merge this for the time being, it's going to be cleaned in a 
>> few days.
> 
> This is OK for me.

Assuming we get some of these earlier patches merged in the next couple 
of days, this will get cleaned up in a larger checkpatch compliance tree 
  which will be the last of the larger.

> 
>>> Instead of this:
>>>
>>> <commit_msg>
>>> From: Brandon Philips <bphilips@xxxxxxx>
>>>
>>> From the author:
>>> "Reviewing the code briefly and saw this.
>>>  You can't change more than DTV_IOCTL_MAX_MSGS at once, not 16."
>>>
>>> Priority: normal
>>>
>>> Signed-off-by: Steven Toth <stoth@xxxxxxxxxxx>
>>> Signed-off-by: Brandon Philips <bphilips@xxxxxxx>
>>> </commit_msg>
>>>
>>> You should just do:
>>>
>>> <commit_msg>
>>> From: Brandon Philips <bphilips@xxxxxxx>
>>>
>>> Reviewing the code briefly and saw this.
>>>  You can't change more than DTV_IOCTL_MAX_MSGS at once, not 16.
>>>
>>> Priority: normal
>>>
>>> Signed-off-by: Brandon Philips <bphilips@xxxxxxx>
>>> Signed-off-by: Steven Toth <stoth@xxxxxxxxxxx>
>>> </commit_msg>
>> Ahh, I always thought this was bottom to top for the chain of 
>> responsibility, with the first sign-off being the person merging the 
>> work. My bad.
> 
> No. The top SOB is the author. The SOB's give the tree sequence that a patch
> took until arriving Linus tree.

Hmm. I see that, now.

> 
> Also, the description is always from the original author POV (generally
> untouched). It is considered a good practice to review the comment (and
> eventually change it) if you think it is not explaining the patch well (for
> example, a patch written by a non-english speaker where people can't understand
> well what is written).
> 
> A good description is a short one line summary of what the patch is doing, and
> a more complete explanation explaining why that patch is needed.
> 
>> I'll change his in any new patches.
>>
>> Is this really a showstopper, or can these cleanups be merged as-is?
> 
> I may eventually pick those patches if you allow me to cherry pick (this means
> that you'll need to re-generate your tree after my pull request). I could
> eventually change this at -git, but, as I'm doing some changes on my merge
> scripts, I prefer to do it at -hg.


it looks like you've already started to merge many of the patches, 
thanks for this.

I'll go through the next set of patches tonight, looking for any obvious 
merge issues prior to submitting the pull req. It should be a little 
smoother next time, especially with patches that came from other people, 
through me.

Thanks again,

- Steve


_______________________________________________
v4l-dvb-maintainer mailing list
v4l-dvb-maintainer@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/v4l-dvb-maintainer