first patch question

first patch question

[Greg Gallagher]

Hi,
   I creating my first patch to the kernel.  I followed the
instruction on the newbies wiki and everything went smoothly.  I got
feedback from the maintainer to fix all the alignment issues in the
file instead of just the one i picked. My question is if I fix 10
alignment issues identified by checkpatch,pl should each fix be a
separate commit? Should I have a 10 commit patch set for the 10
alignment issues or is it better to make them all one commit since all
the commits are fixing alignment issues?

Thanks for our time,

Greg

first patch question

[Nicholas Mc Guire]

On Tue, Aug 07, 2018 at 12:15:04AM -0400, Greg Gallagher wrote:
> Hi,
>    I creating my first patch to the kernel.  I followed the
> instruction on the newbies wiki and everything went smoothly.  I got
> feedback from the maintainer to fix all the alignment issues in the
> file instead of just the one i picked. My question is if I fix 10
> alignment issues identified by checkpatch,pl should each fix be a
> separate commit? Should I have a 10 commit patch set for the 10
> alignment issues or is it better to make them all one commit since all
> the commits are fixing alignment issues?
>
The main criteria I would say is reviewability - so there should be
no need to split it into 10 patches if it is aligment fixes.
If the 10 fixes are in one file I would make one patch out of it.
If it is in files that scripts/get_maintainer.pl suggests different
imaintainer/contributors/reviewers for then put it in seperate
patches - one for each group of maintainers/conributors/reviewers.

thx!
hofrat

first patch question

[greg gallagher]

On 2018-08-07 12:47 AM, Nicholas Mc Guire wrote:
> On Tue, Aug 07, 2018 at 12:15:04AM -0400, Greg Gallagher wrote:
>> Hi,
>>     I creating my first patch to the kernel.  I followed the
>> instruction on the newbies wiki and everything went smoothly.  I got
>> feedback from the maintainer to fix all the alignment issues in the
>> file instead of just the one i picked. My question is if I fix 10
>> alignment issues identified by checkpatch,pl should each fix be a
>> separate commit? Should I have a 10 commit patch set for the 10
>> alignment issues or is it better to make them all one commit since all
>> the commits are fixing alignment issues?
>>
> The main criteria I would say is reviewability - so there should be
> no need to split it into 10 patches if it is aligment fixes.
> If the 10 fixes are in one file I would make one patch out of it.
> If it is in files that scripts/get_maintainer.pl suggests different
> imaintainer/contributors/reviewers for then put it in seperate
> patches - one for each group of maintainers/conributors/reviewers.
>
> thx!
> hofrat
Hi,
 ? Thanks very much for replying.? I took your advice and after an 
unrelated fat finger error I received an acked-by.? Do I have to do 
anything else at this point? Should I add the acked-by to the change 
log? I've read the documentation in the Documents/process directory and 
I can't find a clear answer.

Thanks again for everyone's time,

Greg

first patch question

[Nicholas Mc Guire]

On Thu, Aug 09, 2018 at 01:19:53PM -0400, greg gallagher wrote:
> 
> 
> On 2018-08-07 12:47 AM, Nicholas Mc Guire wrote:
> >On Tue, Aug 07, 2018 at 12:15:04AM -0400, Greg Gallagher wrote:
> >>Hi,
> >>    I creating my first patch to the kernel.  I followed the
> >>instruction on the newbies wiki and everything went smoothly.  I got
> >>feedback from the maintainer to fix all the alignment issues in the
> >>file instead of just the one i picked. My question is if I fix 10
> >>alignment issues identified by checkpatch,pl should each fix be a
> >>separate commit? Should I have a 10 commit patch set for the 10
> >>alignment issues or is it better to make them all one commit since all
> >>the commits are fixing alignment issues?
> >>
> >The main criteria I would say is reviewability - so there should be
> >no need to split it into 10 patches if it is aligment fixes.
> >If the 10 fixes are in one file I would make one patch out of it.
> >If it is in files that scripts/get_maintainer.pl suggests different
> >imaintainer/contributors/reviewers for then put it in seperate
> >patches - one for each group of maintainers/conributors/reviewers.
> >
> >thx!
> >hofrat
> Hi,
> ? Thanks very much for replying.? I took your advice and after an unrelated
> fat finger error I received an acked-by.? Do I have to do anything else at
> this point? Should I add the acked-by to the change log? I've read the
> documentation in the Documents/process directory and I can't find a clear
> answer.
>
Basically all the Acked-by/Review-by/Tested-by are added by
the maintainer that then queues your patch for upstream - so
essentially you are done for this round of fun.

thx!
hofrat

first patch question

[greg gallagher]

On 2018-08-10 01:04 AM, Nicholas Mc Guire wrote:
> On Thu, Aug 09, 2018 at 01:19:53PM -0400, greg gallagher wrote:
>>
>> On 2018-08-07 12:47 AM, Nicholas Mc Guire wrote:
>>> On Tue, Aug 07, 2018 at 12:15:04AM -0400, Greg Gallagher wrote:
>>>> Hi,
>>>>     I creating my first patch to the kernel.  I followed the
>>>> instruction on the newbies wiki and everything went smoothly.  I got
>>>> feedback from the maintainer to fix all the alignment issues in the
>>>> file instead of just the one i picked. My question is if I fix 10
>>>> alignment issues identified by checkpatch,pl should each fix be a
>>>> separate commit? Should I have a 10 commit patch set for the 10
>>>> alignment issues or is it better to make them all one commit since all
>>>> the commits are fixing alignment issues?
>>>>
>>> The main criteria I would say is reviewability - so there should be
>>> no need to split it into 10 patches if it is aligment fixes.
>>> If the 10 fixes are in one file I would make one patch out of it.
>>> If it is in files that scripts/get_maintainer.pl suggests different
>>> imaintainer/contributors/reviewers for then put it in seperate
>>> patches - one for each group of maintainers/conributors/reviewers.
>>>
>>> thx!
>>> hofrat
>> Hi,
>>  ? Thanks very much for replying.? I took your advice and after an unrelated
>> fat finger error I received an acked-by.? Do I have to do anything else at
>> this point? Should I add the acked-by to the change log? I've read the
>> documentation in the Documents/process directory and I can't find a clear
>> answer.
>>
> Basically all the Acked-by/Review-by/Tested-by are added by
> the maintainer that then queues your patch for upstream - so
> essentially you are done for this round of fun.
>
> thx!
> hofrat
Awesome, thanks very much for you reply.? Greatly appreciate the help:)