Re: [Outreachy] Wiki updated needed - Patchset Subject

Re: [Outreachy] Wiki updated needed - Patchset Subject

[Deepak R Varma]

On Sun, Oct 23, 2022 at 06:51:11PM -0700, Alison Schofield wrote:
> Greg KH has mentioned this a couple of times lately -
>
> >> Any reason you do not have "staging: vt6655:" as the prefix for
> >> this 0/X email subject line?
> >>
> >> thanks,
> >>
> >> greg k-h
>
> A quick peek in the tutorial tells me it could be made clearer.
> It needs to explicitly say the prefix is required, and then
> make sure any examples do it.

Hello Alison,
There already is a section named "Patch Subject formatting" on this link [1]
with enough description to determine the patch subject prefix. Can you please
review and confirm where we should improve further to make it clearer??

	[1] https://kernelnewbies.org/PatchPhilosophy

Thank you,
./drv

>
> Thanks,
> Alison
>

[Outreachy] Wiki updated needed - Patchset Subject

[Alison Schofield]

Greg KH has mentioned this a couple of times lately -

>> Any reason you do not have "staging: vt6655:" as the prefix for
>> this 0/X email subject line?
>> 
>> thanks,
>> 
>> greg k-h

A quick peek in the tutorial tells me it could be made clearer.
It needs to explicitly say the prefix is required, and then
make sure any examples do it.

Thanks,
Alison

Re: [Outreachy] Wiki updated needed - Patchset Subject

[Alison Schofield]

On Sat, Oct 22, 2022 at 10:59:38AM +0530, Deepak R Varma wrote:
> On Sun, Oct 23, 2022 at 06:51:11PM -0700, Alison Schofield wrote:
> > Greg KH has mentioned this a couple of times lately -
> >
> > >> Any reason you do not have "staging: vt6655:" as the prefix for
> > >> this 0/X email subject line?
> > >>
> > >> thanks,
> > >>
> > >> greg k-h
> >
> > A quick peek in the tutorial tells me it could be made clearer.
> > It needs to explicitly say the prefix is required, and then
> > make sure any examples do it.
> 
> Hello Alison,
> There already is a section named "Patch Subject formatting" on this link [1]
> with enough description to determine the patch subject prefix. Can you please
> review and confirm where we should improve further to make it clearer??
> 
> 	[1] https://kernelnewbies.org/PatchPhilosophy
> 
> Thank you,
> ./drv


In https://kernelnewbies.org/Outreachyfirstpatch

The example cover letter in 'Submitting a Patchset' has the
subject "[RFC 0/7] Update REPORTING-BUGS", with no subsys.
It is also confusing because the cover letter is an RFC and
the patches are a mix of RFC and PATCH. Very atypical.

Let's replace this example with one that was actually submitted
and accepted by an Outreachy Intern.  No slight intended to the
existing set as it contains an interesting set of changes. However,
an example for newbies might better come from newbies.

How about a patchset off this list? 

Would you mind using this one of yours:
[PATCH v4 00/11] staging: r8188eu: trivial code cleanup patches
but trim it to 3-4 patches plus the cover.

It's not a trivial edit, because you need to replace all the
example steps with the new patchset content.

Thanks,
Alison

> 
> >
> > Thanks,
> > Alison
> >
> 
> 

Re: [Outreachy] Wiki updated needed - Patchset Subject

[Deepak R Varma]

On Mon, Oct 24, 2022 at 02:12:46PM -0700, Alison Schofield wrote:
> On Sat, Oct 22, 2022 at 10:59:38AM +0530, Deepak R Varma wrote:
> > On Sun, Oct 23, 2022 at 06:51:11PM -0700, Alison Schofield wrote:
> > > Greg KH has mentioned this a couple of times lately -
> > >
> > > >> Any reason you do not have "staging: vt6655:" as the prefix for
> > > >> this 0/X email subject line?
> > > >>
> > > >> thanks,
> > > >>
> > > >> greg k-h
> > >
> > > A quick peek in the tutorial tells me it could be made clearer.
> > > It needs to explicitly say the prefix is required, and then
> > > make sure any examples do it.
> >
> > Hello Alison,
> > There already is a section named "Patch Subject formatting" on this link [1]
> > with enough description to determine the patch subject prefix. Can you please
> > review and confirm where we should improve further to make it clearer??
> >
> > 	[1] https://kernelnewbies.org/PatchPhilosophy
> >
> > Thank you,
> > ./drv
>
>
> In https://kernelnewbies.org/Outreachyfirstpatch
>
> The example cover letter in 'Submitting a Patchset' has the
> subject "[RFC 0/7] Update REPORTING-BUGS", with no subsys.
> It is also confusing because the cover letter is an RFC and
> the patches are a mix of RFC and PATCH. Very atypical.
>
> Let's replace this example with one that was actually submitted
> and accepted by an Outreachy Intern.  No slight intended to the
> existing set as it contains an interesting set of changes. However,
> an example for newbies might better come from newbies.
>
> How about a patchset off this list?
>
> Would you mind using this one of yours:
> [PATCH v4 00/11] staging: r8188eu: trivial code cleanup patches
> but trim it to 3-4 patches plus the cover.
>
> It's not a trivial edit, because you need to replace all the
> example steps with the new patchset content.

Sounds good. I will work on it and share my proposed changes for your review.

./drv

>
> Thanks,
> Alison
>
> >
> > >
> > > Thanks,
> > > Alison
> > >
> >
> >

Re: [Outreachy] Wiki updated needed - Patchset Subject

[Deepak R Varma]

On Mon, Oct 24, 2022 at 02:12:46PM -0700, Alison Schofield wrote:
> On Sat, Oct 22, 2022 at 10:59:38AM +0530, Deepak R Varma wrote:
> > On Sun, Oct 23, 2022 at 06:51:11PM -0700, Alison Schofield wrote:
> > > Greg KH has mentioned this a couple of times lately -
> > >
> > > >> Any reason you do not have "staging: vt6655:" as the prefix for
> > > >> this 0/X email subject line?
> > > >>
> > > >> thanks,
> > > >>
> > > >> greg k-h
> > >
> > > A quick peek in the tutorial tells me it could be made clearer.
> > > It needs to explicitly say the prefix is required, and then
> > > make sure any examples do it.
> >
> > Hello Alison,
> > There already is a section named "Patch Subject formatting" on this link [1]
> > with enough description to determine the patch subject prefix. Can you please
> > review and confirm where we should improve further to make it clearer??
> >
> > 	[1] https://kernelnewbies.org/PatchPhilosophy
> >
> > Thank you,
> > ./drv
>
>
> In https://kernelnewbies.org/Outreachyfirstpatch
>
> The example cover letter in 'Submitting a Patchset' has the
> subject "[RFC 0/7] Update REPORTING-BUGS", with no subsys.
> It is also confusing because the cover letter is an RFC and
> the patches are a mix of RFC and PATCH. Very atypical.
>
> Let's replace this example with one that was actually submitted
> and accepted by an Outreachy Intern.  No slight intended to the
> existing set as it contains an interesting set of changes. However,
> an example for newbies might better come from newbies.
>
> How about a patchset off this list?
>
> Would you mind using this one of yours:
> [PATCH v4 00/11] staging: r8188eu: trivial code cleanup patches
> but trim it to 3-4 patches plus the cover.
>
> It's not a trivial edit, because you need to replace all the
> example steps with the new patchset content.

Hello Alison,
I made the changes as suggested and saved those. Looks like they might have
already gone to production. Sorry about that. I was thinking they will be
submitted to a moderator for review before releasing,

Could you please review and let me know if any edits are required.

Thank you,
./drv

>
> Thanks,
> Alison
>
> >
> > >
> > > Thanks,
> > > Alison
> > >
> >
> >

Re: [Outreachy] Wiki updated needed - Patchset Subject

[Deepak R Varma]

On Wed, Oct 26, 2022 at 04:00:58AM +0530, Deepak Varma wrote:
> On Mon, Oct 24, 2022 at 02:12:46PM -0700, Alison Schofield wrote:
> > On Sat, Oct 22, 2022 at 10:59:38AM +0530, Deepak R Varma wrote:
> > > On Sun, Oct 23, 2022 at 06:51:11PM -0700, Alison Schofield wrote:
> > > > Greg KH has mentioned this a couple of times lately -
> > > >
> > > > >> Any reason you do not have "staging: vt6655:" as the prefix for
> > > > >> this 0/X email subject line?
> > > > >>
> > > > >> thanks,
> > > > >>
> > > > >> greg k-h
> > > >
> > > > A quick peek in the tutorial tells me it could be made clearer.
> > > > It needs to explicitly say the prefix is required, and then
> > > > make sure any examples do it.
> > >
> > > Hello Alison,
> > > There already is a section named "Patch Subject formatting" on this link [1]
> > > with enough description to determine the patch subject prefix. Can you please
> > > review and confirm where we should improve further to make it clearer??
> > >
> > > 	[1] https://kernelnewbies.org/PatchPhilosophy
> > >
> > > Thank you,
> > > ./drv
> >
> >
> > In https://kernelnewbies.org/Outreachyfirstpatch
> >
> > The example cover letter in 'Submitting a Patchset' has the
> > subject "[RFC 0/7] Update REPORTING-BUGS", with no subsys.
> > It is also confusing because the cover letter is an RFC and
> > the patches are a mix of RFC and PATCH. Very atypical.
> >
> > Let's replace this example with one that was actually submitted
> > and accepted by an Outreachy Intern.  No slight intended to the
> > existing set as it contains an interesting set of changes. However,
> > an example for newbies might better come from newbies.
> >
> > How about a patchset off this list?
> >
> > Would you mind using this one of yours:
> > [PATCH v4 00/11] staging: r8188eu: trivial code cleanup patches
> > but trim it to 3-4 patches plus the cover.
> >
> > It's not a trivial edit, because you need to replace all the
> > example steps with the new patchset content.
>
> Hello Alison,
> I made the changes as suggested and saved those. Looks like they might have
> already gone to production. Sorry about that. I was thinking they will be
> submitted to a moderator for review before releasing,
>
> Could you please review and let me know if any edits are required.

Hello Alison,
Did you get a chance to review the edits I made? Please share your feedback so
that I can proceed with the next changes to the tutorial page.

Thank you,
./drv

>
> Thank you,
> ./drv
>
> >
> > Thanks,
> > Alison
> >
> > >
> > > >
> > > > Thanks,
> > > > Alison
> > > >
> > >
> > >

Re: [Outreachy] Wiki updated needed - Patchset Subject

[Alison Schofield]

On Fri, Oct 28, 2022 at 03:36:07AM +0530, Deepak R Varma wrote:
> On Wed, Oct 26, 2022 at 04:00:58AM +0530, Deepak Varma wrote:
> > On Mon, Oct 24, 2022 at 02:12:46PM -0700, Alison Schofield wrote:
> > > On Sat, Oct 22, 2022 at 10:59:38AM +0530, Deepak R Varma wrote:
> > > > On Sun, Oct 23, 2022 at 06:51:11PM -0700, Alison Schofield wrote:
> > > > > Greg KH has mentioned this a couple of times lately -
> > > > >
> > > > > >> Any reason you do not have "staging: vt6655:" as the prefix for
> > > > > >> this 0/X email subject line?
> > > > > >>
> > > > > >> thanks,
> > > > > >>
> > > > > >> greg k-h
> > > > >
> > > > > A quick peek in the tutorial tells me it could be made clearer.
> > > > > It needs to explicitly say the prefix is required, and then
> > > > > make sure any examples do it.
> > > >
> > > > Hello Alison,
> > > > There already is a section named "Patch Subject formatting" on this link [1]
> > > > with enough description to determine the patch subject prefix. Can you please
> > > > review and confirm where we should improve further to make it clearer??
> > > >
> > > > 	[1] https://kernelnewbies.org/PatchPhilosophy
> > > >
> > > > Thank you,
> > > > ./drv
> > >
> > >
> > > In https://kernelnewbies.org/Outreachyfirstpatch
> > >
> > > The example cover letter in 'Submitting a Patchset' has the
> > > subject "[RFC 0/7] Update REPORTING-BUGS", with no subsys.
> > > It is also confusing because the cover letter is an RFC and
> > > the patches are a mix of RFC and PATCH. Very atypical.
> > >
> > > Let's replace this example with one that was actually submitted
> > > and accepted by an Outreachy Intern.  No slight intended to the
> > > existing set as it contains an interesting set of changes. However,
> > > an example for newbies might better come from newbies.
> > >
> > > How about a patchset off this list?
> > >
> > > Would you mind using this one of yours:
> > > [PATCH v4 00/11] staging: r8188eu: trivial code cleanup patches
> > > but trim it to 3-4 patches plus the cover.
> > >
> > > It's not a trivial edit, because you need to replace all the
> > > example steps with the new patchset content.
> >
> > Hello Alison,
> > I made the changes as suggested and saved those. Looks like they might have
> > already gone to production. Sorry about that. I was thinking they will be
> > submitted to a moderator for review before releasing,
> >
> > Could you please review and let me know if any edits are required.
> 
> Hello Alison,
> Did you get a chance to review the edits I made? Please share your feedback so
> that I can proceed with the next changes to the tutorial page.
> 
> Thank you,
> ./drv

Thanks for doing that! I see you were very thorough :)

A couple of thoughts and then a long suggestion:

- Can you trim it to a smaller patchset?  
It's OK if you just edit it. It does not need to reflect reality.
The reason I ask is because a ten patch set is not the example we
want to set for newbies. It's the stretch goal, not the norm.

- Your commit messages would be better if they were consistent,
either starting with upper or lower case, but not mixed.
Usually there is a pattern set in the files, but alas, when I
look at the pretty oneline of rtw_br_ext.c, there is no pattern.
(and it's messy)

Seque to another tutorial improvement - explaining the point
of pretty oneline, and examples would help.

The point, is to make it easy on the eyes of the developer.
When developers scan through the history of work on a file,
it is easier to do when the appearance is consistent. 

It may not seem important to the task at hand for a newbie,
but the consistency pays off when you are staring at these
patches day in and day out.

You'll want to alias it because you'll use it often:
alias gitpretty='git log --pretty=oneline --abbrev-commit'

In 'Viewing your commit' it says 'You'll also want to make sure
your commit looks fine when you run these two commands:"

Let's put some more detail around 'looks fine':
- prefix is correct
- line does not wrap on 80 Column screen
- styling matches other commits in that file with respect to use
  of punctuation, upper/lower case. Subsystems and files have
  different styles (see examples below). The point it to respect
  the style.

This is fine:
drivers/staging/iio/adc$ gitpretty ad7816.c  (edited for fineness)

f1b753a0f866 staging: iio: ad7816: Drop unnecessary initialization of variables
c24a4173f6bb staging: iio: ad7816: Add device tree table
72e3a5248da9 staging: iio: ad7816: Set RD/WR pin and CONVST pin as outputs
06c77f564ddb staging: iio: ad7816: Do not use busy_pin in case of AD7818
073a391ca035 staging: iio: ad7816: Switch to the gpio descriptor interface

This is *not* fine:
(line wrap, prefixes and upper/lower case are inconsistent
/staging/most/video$ gitpretty video.c  (trimmed to illustrate)

d6ef48e59582 staging: most: video: Make use of the helper macro LIST_HEAD()
fa8db3989362 staging/most: Remove all strcpy() uses in favor of strscpy()
6367dee9e3db staging: most: Switch from strlcpy to strscpy
b27652753918 staging: most: move core files out of the staging area
e653614ee183 media: staging/most: rename VFL_TYPE_GRABBER to _VIDEO
08283d307444 staging: most: block module removal while having active configfs items
a20eefaee646 staging: most: Use DEFINE_SPINLOCK() instead of struct spinlock
338d9637361c staging/most/video: set device_caps in struct video_device

This info is not focused patchsets. Every patch that comes across this
list should make best effort to be 'gitpretty'.

If you are out of time Deeva, perhaps percolate this up on the
list for someone else to pick up.

Thanks,
Alison
> 
> >
> > Thank you,
> > ./drv
> >
> > >
> > > Thanks,
> > > Alison
> > >
> > > >
> > > > >
> > > > > Thanks,
> > > > > Alison
> > > > >
> > > >
> > > >
> 
> 

Re: [Outreachy] Wiki updated needed - Patchset Subject

[Deepak R Varma]

On Thu, Oct 27, 2022 at 07:33:19PM -0700, Alison Schofield wrote:
> On Fri, Oct 28, 2022 at 03:36:07AM +0530, Deepak R Varma wrote:
> > On Wed, Oct 26, 2022 at 04:00:58AM +0530, Deepak Varma wrote:
> > > On Mon, Oct 24, 2022 at 02:12:46PM -0700, Alison Schofield wrote:
> > > > In https://kernelnewbies.org/Outreachyfirstpatch
> > > >
> > > > The example cover letter in 'Submitting a Patchset' has the
> > > > subject "[RFC 0/7] Update REPORTING-BUGS", with no subsys.
> > > > It is also confusing because the cover letter is an RFC and
> > > > the patches are a mix of RFC and PATCH. Very atypical.
> > > >
> > > > Let's replace this example with one that was actually submitted
> > > > and accepted by an Outreachy Intern.  No slight intended to the
> > > > existing set as it contains an interesting set of changes. However,
> > > > an example for newbies might better come from newbies.
> > > >
> > > > How about a patchset off this list?
> > > >
> > > > Would you mind using this one of yours:
> > > > [PATCH v4 00/11] staging: r8188eu: trivial code cleanup patches
> > > > but trim it to 3-4 patches plus the cover.
> > > >
> > > > It's not a trivial edit, because you need to replace all the
> > > > example steps with the new patchset content.
> > >
> > > Hello Alison,
> > > I made the changes as suggested and saved those. Looks like they might have
> > > already gone to production. Sorry about that. I was thinking they will be
> > > submitted to a moderator for review before releasing,
> > >
> > > Could you please review and let me know if any edits are required.
> >
> > Hello Alison,
> > Did you get a chance to review the edits I made? Please share your feedback so
> > that I can proceed with the next changes to the tutorial page.
> >
> > Thank you,
> > ./drv
>
> Thanks for doing that! I see you were very thorough :)
>
> A couple of thoughts and then a long suggestion:
>
> - Can you trim it to a smaller patchset?
> It's OK if you just edit it. It does not need to reflect reality.
> The reason I ask is because a ten patch set is not the example we
> want to set for newbies. It's the stretch goal, not the norm.

Agreed. I made the modifications and they are ready for your review.
>
> - Your commit messages would be better if they were consistent,
> either starting with upper or lower case, but not mixed.
> Usually there is a pattern set in the files, but alas, when I
> look at the pretty oneline of rtw_br_ext.c, there is no pattern.
> (and it's messy)

My apologies for the mixed case commits. I will be careful with the styles going
forward. Thank you for catching that.
For the purpose of proposed edits, I have edited the example per your observations.

>
> Seque to another tutorial improvement - explaining the point
> of pretty oneline, and examples would help.
>
> The point, is to make it easy on the eyes of the developer.
> When developers scan through the history of work on a file,
> it is easier to do when the appearance is consistent.
>
> It may not seem important to the task at hand for a newbie,
> but the consistency pays off when you are staring at these
> patches day in and day out.
>
> You'll want to alias it because you'll use it often:
> alias gitpretty='git log --pretty=oneline --abbrev-commit'
>
> In 'Viewing your commit' it says 'You'll also want to make sure
> your commit looks fine when you run these two commands:"
>
> Let's put some more detail around 'looks fine':
> - prefix is correct
> - line does not wrap on 80 Column screen
> - styling matches other commits in that file with respect to use
>   of punctuation, upper/lower case. Subsystems and files have
>   different styles (see examples below). The point it to respect
>   the style.
>
> This is fine:
> drivers/staging/iio/adc$ gitpretty ad7816.c  (edited for fineness)
>
> f1b753a0f866 staging: iio: ad7816: Drop unnecessary initialization of variables
> c24a4173f6bb staging: iio: ad7816: Add device tree table
> 72e3a5248da9 staging: iio: ad7816: Set RD/WR pin and CONVST pin as outputs
> 06c77f564ddb staging: iio: ad7816: Do not use busy_pin in case of AD7818
> 073a391ca035 staging: iio: ad7816: Switch to the gpio descriptor interface
>
> This is *not* fine:
> (line wrap, prefixes and upper/lower case are inconsistent
> /staging/most/video$ gitpretty video.c  (trimmed to illustrate)
>
> d6ef48e59582 staging: most: video: Make use of the helper macro LIST_HEAD()
> fa8db3989362 staging/most: Remove all strcpy() uses in favor of strscpy()
> 6367dee9e3db staging: most: Switch from strlcpy to strscpy
> b27652753918 staging: most: move core files out of the staging area
> e653614ee183 media: staging/most: rename VFL_TYPE_GRABBER to _VIDEO
> 08283d307444 staging: most: block module removal while having active configfs items
> a20eefaee646 staging: most: Use DEFINE_SPINLOCK() instead of struct spinlock
> 338d9637361c staging/most/video: set device_caps in struct video_device
>
> This info is not focused patchsets. Every patch that comes across this
> list should make best effort to be 'gitpretty'.
>
> If you are out of time Deeva, perhaps percolate this up on the
> list for someone else to pick up.

I get your point and can relate to your suggestion based on how often I need to
use the pretty command. Following suggestion will definitely make things better
visually for the developers.

I am thinking we should add a new sections, may be "Perform the gitpretty checks"
somewhere before "submit patch" section and include the verbiage and examples
(both the good and the bad example) for better understanding.
Does that sound good to you?

./drv

>
> Thanks,
> Alison
> >
> > >
> > > Thank you,
> > > ./drv
> > >
> > > >
> > > > Thanks,
> > > > Alison
> > > >
> > > > >
> > > > > >
> > > > > > Thanks,
> > > > > > Alison
> > > > > >
> > > > >
> > > > >
> >
> >
>

Re: [Outreachy] Wiki updated needed - Patchset Subject

[Alison Schofield]

On Sat, Oct 29, 2022 at 12:29:39AM +0530, Deepak R Varma wrote:
> On Thu, Oct 27, 2022 at 07:33:19PM -0700, Alison Schofield wrote:
> > On Fri, Oct 28, 2022 at 03:36:07AM +0530, Deepak R Varma wrote:
> > > On Wed, Oct 26, 2022 at 04:00:58AM +0530, Deepak Varma wrote:
> > > > On Mon, Oct 24, 2022 at 02:12:46PM -0700, Alison Schofield wrote:
> > > > > In https://kernelnewbies.org/Outreachyfirstpatch
> > > > >
> > > > > The example cover letter in 'Submitting a Patchset' has the
> > > > > subject "[RFC 0/7] Update REPORTING-BUGS", with no subsys.
> > > > > It is also confusing because the cover letter is an RFC and
> > > > > the patches are a mix of RFC and PATCH. Very atypical.
> > > > >
> > > > > Let's replace this example with one that was actually submitted
> > > > > and accepted by an Outreachy Intern.  No slight intended to the
> > > > > existing set as it contains an interesting set of changes. However,
> > > > > an example for newbies might better come from newbies.
> > > > >
> > > > > How about a patchset off this list?
> > > > >
> > > > > Would you mind using this one of yours:
> > > > > [PATCH v4 00/11] staging: r8188eu: trivial code cleanup patches
> > > > > but trim it to 3-4 patches plus the cover.
> > > > >
> > > > > It's not a trivial edit, because you need to replace all the
> > > > > example steps with the new patchset content.
> > > >
> > > > Hello Alison,
> > > > I made the changes as suggested and saved those. Looks like they might have
> > > > already gone to production. Sorry about that. I was thinking they will be
> > > > submitted to a moderator for review before releasing,
> > > >
> > > > Could you please review and let me know if any edits are required.
> > >
> > > Hello Alison,
> > > Did you get a chance to review the edits I made? Please share your feedback so
> > > that I can proceed with the next changes to the tutorial page.
> > >
> > > Thank you,
> > > ./drv
> >
> > Thanks for doing that! I see you were very thorough :)
> >
> > A couple of thoughts and then a long suggestion:
> >
> > - Can you trim it to a smaller patchset?
> > It's OK if you just edit it. It does not need to reflect reality.
> > The reason I ask is because a ten patch set is not the example we
> > want to set for newbies. It's the stretch goal, not the norm.
> 
> Agreed. I made the modifications and they are ready for your review.
Thanks
> >
> > - Your commit messages would be better if they were consistent,
> > either starting with upper or lower case, but not mixed.
> > Usually there is a pattern set in the files, but alas, when I
> > look at the pretty oneline of rtw_br_ext.c, there is no pattern.
> > (and it's messy)
> 
> My apologies for the mixed case commits. I will be careful with the styles going
> forward. Thank you for catching that.
> For the purpose of proposed edits, I have edited the example per your observations.
> 
Thanks
> >
> > Seque to another tutorial improvement - explaining the point
> > of pretty oneline, and examples would help.
> >
> > The point, is to make it easy on the eyes of the developer.
> > When developers scan through the history of work on a file,
> > it is easier to do when the appearance is consistent.
> >
> > It may not seem important to the task at hand for a newbie,
> > but the consistency pays off when you are staring at these
> > patches day in and day out.
> >
> > You'll want to alias it because you'll use it often:
> > alias gitpretty='git log --pretty=oneline --abbrev-commit'
> >
> > In 'Viewing your commit' it says 'You'll also want to make sure
> > your commit looks fine when you run these two commands:"
> >
> > Let's put some more detail around 'looks fine':
> > - prefix is correct
> > - line does not wrap on 80 Column screen
> > - styling matches other commits in that file with respect to use
> >   of punctuation, upper/lower case. Subsystems and files have
> >   different styles (see examples below). The point it to respect
> >   the style.
> >
> > This is fine:
> > drivers/staging/iio/adc$ gitpretty ad7816.c  (edited for fineness)
> >
> > f1b753a0f866 staging: iio: ad7816: Drop unnecessary initialization of variables
> > c24a4173f6bb staging: iio: ad7816: Add device tree table
> > 72e3a5248da9 staging: iio: ad7816: Set RD/WR pin and CONVST pin as outputs
> > 06c77f564ddb staging: iio: ad7816: Do not use busy_pin in case of AD7818
> > 073a391ca035 staging: iio: ad7816: Switch to the gpio descriptor interface
> >
> > This is *not* fine:
> > (line wrap, prefixes and upper/lower case are inconsistent
> > /staging/most/video$ gitpretty video.c  (trimmed to illustrate)
> >
> > d6ef48e59582 staging: most: video: Make use of the helper macro LIST_HEAD()
> > fa8db3989362 staging/most: Remove all strcpy() uses in favor of strscpy()
> > 6367dee9e3db staging: most: Switch from strlcpy to strscpy
> > b27652753918 staging: most: move core files out of the staging area
> > e653614ee183 media: staging/most: rename VFL_TYPE_GRABBER to _VIDEO
> > 08283d307444 staging: most: block module removal while having active configfs items
> > a20eefaee646 staging: most: Use DEFINE_SPINLOCK() instead of struct spinlock
> > 338d9637361c staging/most/video: set device_caps in struct video_device
> >
> > This info is not focused patchsets. Every patch that comes across this
> > list should make best effort to be 'gitpretty'.
> >
> > If you are out of time Deeva, perhaps percolate this up on the
> > list for someone else to pick up.
> 
> I get your point and can relate to your suggestion based on how often I need to
> use the pretty command. Following suggestion will definitely make things better
> visually for the developers.
> 
> I am thinking we should add a new sections, may be "Perform the gitpretty checks"
> somewhere before "submit patch" section and include the verbiage and examples
> (both the good and the bad example) for better understanding.
> Does that sound good to you?

Yes. Deserves it's only section!

We (esp me) in Outreachy are picky about this because newbies are
hopping all over staging, and by doing so, they are touching the code
of numerous subsystems, with different maintainers, and different
styles. We want to adhere to the style of the subsystem we are visiting.

Thanks!

> > ./drv > 
> >
> > Thanks,
> > Alison
> > >
> > > >
> > > > Thank you,
> > > > ./drv
> > > >
> > > > >
> > > > > Thanks,
> > > > > Alison
> > > > >
> > > > > >
> > > > > > >
> > > > > > > Thanks,
> > > > > > > Alison
> > > > > > >
> > > > > >
> > > > > >
> > >
> > >
> >
> 
> 

Re: [Outreachy] Wiki updated needed - Patchset Subject

[Deepak R Varma]

On Fri, Oct 28, 2022 at 01:32:41PM -0700, Alison Schofield wrote:
> On Sat, Oct 29, 2022 at 12:29:39AM +0530, Deepak R Varma wrote:
> > On Thu, Oct 27, 2022 at 07:33:19PM -0700, Alison Schofield wrote:
> > > On Fri, Oct 28, 2022 at 03:36:07AM +0530, Deepak R Varma wrote:
> > > > On Wed, Oct 26, 2022 at 04:00:58AM +0530, Deepak Varma wrote:
> > > > > On Mon, Oct 24, 2022 at 02:12:46PM -0700, Alison Schofield wrote:
> > > > > > In https://kernelnewbies.org/Outreachyfirstpatch
> > > > > >
> > > > > > The example cover letter in 'Submitting a Patchset' has the
> > > > > > subject "[RFC 0/7] Update REPORTING-BUGS", with no subsys.
> > > > > > It is also confusing because the cover letter is an RFC and
> > > > > > the patches are a mix of RFC and PATCH. Very atypical.
> > > > > >
> > > > > > Let's replace this example with one that was actually submitted
> > > > > > and accepted by an Outreachy Intern.  No slight intended to the
> > > > > > existing set as it contains an interesting set of changes. However,
> > > > > > an example for newbies might better come from newbies.
> > > > > >
> > > > > > How about a patchset off this list?
> > > > > >
> > > > > > Would you mind using this one of yours:
> > > > > > [PATCH v4 00/11] staging: r8188eu: trivial code cleanup patches
> > > > > > but trim it to 3-4 patches plus the cover.
> > > > > >
> > > > > > It's not a trivial edit, because you need to replace all the
> > > > > > example steps with the new patchset content.
> > > > >
> > > > > Hello Alison,
> > > > > I made the changes as suggested and saved those. Looks like they might have
> > > > > already gone to production. Sorry about that. I was thinking they will be
> > > > > submitted to a moderator for review before releasing,
> > > > >
> > > > > Could you please review and let me know if any edits are required.
> > > >
> > > > Hello Alison,
> > > > Did you get a chance to review the edits I made? Please share your feedback so
> > > > that I can proceed with the next changes to the tutorial page.
> > > >
> > > > Thank you,
> > > > ./drv
> > >
> > > Thanks for doing that! I see you were very thorough :)
> > >
> > > A couple of thoughts and then a long suggestion:
> > >
> > > - Can you trim it to a smaller patchset?
> > > It's OK if you just edit it. It does not need to reflect reality.
> > > The reason I ask is because a ten patch set is not the example we
> > > want to set for newbies. It's the stretch goal, not the norm.
> >
> > Agreed. I made the modifications and they are ready for your review.
> Thanks
> > >
> > > - Your commit messages would be better if they were consistent,
> > > either starting with upper or lower case, but not mixed.
> > > Usually there is a pattern set in the files, but alas, when I
> > > look at the pretty oneline of rtw_br_ext.c, there is no pattern.
> > > (and it's messy)
> >
> > My apologies for the mixed case commits. I will be careful with the styles going
> > forward. Thank you for catching that.
> > For the purpose of proposed edits, I have edited the example per your observations.
> >
> Thanks
> > >
> > > Seque to another tutorial improvement - explaining the point
> > > of pretty oneline, and examples would help.
> > >
> > > The point, is to make it easy on the eyes of the developer.
> > > When developers scan through the history of work on a file,
> > > it is easier to do when the appearance is consistent.
> > >
> > > It may not seem important to the task at hand for a newbie,
> > > but the consistency pays off when you are staring at these
> > > patches day in and day out.
> > >
> > > You'll want to alias it because you'll use it often:
> > > alias gitpretty='git log --pretty=oneline --abbrev-commit'
> > >
> > > In 'Viewing your commit' it says 'You'll also want to make sure
> > > your commit looks fine when you run these two commands:"
> > >
> > > Let's put some more detail around 'looks fine':
> > > - prefix is correct
> > > - line does not wrap on 80 Column screen
> > > - styling matches other commits in that file with respect to use
> > >   of punctuation, upper/lower case. Subsystems and files have
> > >   different styles (see examples below). The point it to respect
> > >   the style.
> > >
> > > This is fine:
> > > drivers/staging/iio/adc$ gitpretty ad7816.c  (edited for fineness)
> > >
> > > f1b753a0f866 staging: iio: ad7816: Drop unnecessary initialization of variables
> > > c24a4173f6bb staging: iio: ad7816: Add device tree table
> > > 72e3a5248da9 staging: iio: ad7816: Set RD/WR pin and CONVST pin as outputs
> > > 06c77f564ddb staging: iio: ad7816: Do not use busy_pin in case of AD7818
> > > 073a391ca035 staging: iio: ad7816: Switch to the gpio descriptor interface
> > >
> > > This is *not* fine:
> > > (line wrap, prefixes and upper/lower case are inconsistent
> > > /staging/most/video$ gitpretty video.c  (trimmed to illustrate)
> > >
> > > d6ef48e59582 staging: most: video: Make use of the helper macro LIST_HEAD()
> > > fa8db3989362 staging/most: Remove all strcpy() uses in favor of strscpy()
> > > 6367dee9e3db staging: most: Switch from strlcpy to strscpy
> > > b27652753918 staging: most: move core files out of the staging area
> > > e653614ee183 media: staging/most: rename VFL_TYPE_GRABBER to _VIDEO
> > > 08283d307444 staging: most: block module removal while having active configfs items
> > > a20eefaee646 staging: most: Use DEFINE_SPINLOCK() instead of struct spinlock
> > > 338d9637361c staging/most/video: set device_caps in struct video_device
> > >
> > > This info is not focused patchsets. Every patch that comes across this
> > > list should make best effort to be 'gitpretty'.
> > >
> > > If you are out of time Deeva, perhaps percolate this up on the
> > > list for someone else to pick up.
> >
> > I get your point and can relate to your suggestion based on how often I need to
> > use the pretty command. Following suggestion will definitely make things better
> > visually for the developers.
> >
> > I am thinking we should add a new sections, may be "Perform the gitpretty checks"
> > somewhere before "submit patch" section and include the verbiage and examples
> > (both the good and the bad example) for better understanding.
> > Does that sound good to you?
>
> Yes. Deserves it's only section!
>
> We (esp me) in Outreachy are picky about this because newbies are
> hopping all over staging, and by doing so, they are touching the code
> of numerous subsystems, with different maintainers, and different
> styles. We want to adhere to the style of the subsystem we are visiting.

Hello Alison,
I have attempted to introduce a new section and supporting information to
accomplish guidelines on following driver style for patch work. Could you please
and suggest if any edits are required.

New section: "Following the Driver commit style"

Thank you,
./drv

>
> Thanks!
>
> > > ./drv >
> > >
> > > Thanks,
> > > Alison
> > > >
> > > > >
> > > > > Thank you,
> > > > > ./drv
> > > > >
> > > > > >
> > > > > > Thanks,
> > > > > > Alison
> > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > Thanks,
> > > > > > > > Alison
> > > > > > > >
> > > > > > >
> > > > > > >
> > > >
> > > >
> > >
> >
> >
>

Re: [Outreachy] Wiki updated needed - Patchset Subject

[Deepak R Varma]

On Tue, Nov 01, 2022 at 02:45:41AM +0530, Deepak Varma wrote:
> On Fri, Oct 28, 2022 at 01:32:41PM -0700, Alison Schofield wrote:
> > On Sat, Oct 29, 2022 at 12:29:39AM +0530, Deepak R Varma wrote:
> > > On Thu, Oct 27, 2022 at 07:33:19PM -0700, Alison Schofield wrote:
> > > > On Fri, Oct 28, 2022 at 03:36:07AM +0530, Deepak R Varma wrote:
> > > > > On Wed, Oct 26, 2022 at 04:00:58AM +0530, Deepak Varma wrote:
> > > > > > On Mon, Oct 24, 2022 at 02:12:46PM -0700, Alison Schofield wrote:
> > > > > > > In https://kernelnewbies.org/Outreachyfirstpatch
> > > > > > >
> > > > > > > The example cover letter in 'Submitting a Patchset' has the
> > > > > > > subject "[RFC 0/7] Update REPORTING-BUGS", with no subsys.
> > > > > > > It is also confusing because the cover letter is an RFC and
> > > > > > > the patches are a mix of RFC and PATCH. Very atypical.
> > > > > > >
> > > > > > > Let's replace this example with one that was actually submitted
> > > > > > > and accepted by an Outreachy Intern.  No slight intended to the
> > > > > > > existing set as it contains an interesting set of changes. However,
> > > > > > > an example for newbies might better come from newbies.
> > > > > > >
> > > > > > > How about a patchset off this list?
> > > > > > >
> > > > > > > Would you mind using this one of yours:
> > > > > > > [PATCH v4 00/11] staging: r8188eu: trivial code cleanup patches
> > > > > > > but trim it to 3-4 patches plus the cover.
> > > > > > >
> > > > > > > It's not a trivial edit, because you need to replace all the
> > > > > > > example steps with the new patchset content.
> > > > > >
> > > > > > Hello Alison,
> > > > > > I made the changes as suggested and saved those. Looks like they might have
> > > > > > already gone to production. Sorry about that. I was thinking they will be
> > > > > > submitted to a moderator for review before releasing,
> > > > > >
> > > > > > Could you please review and let me know if any edits are required.
> > > > >
> > > > > Hello Alison,
> > > > > Did you get a chance to review the edits I made? Please share your feedback so
> > > > > that I can proceed with the next changes to the tutorial page.
> > > > >
> > > > > Thank you,
> > > > > ./drv
> > > >
> > > > Thanks for doing that! I see you were very thorough :)
> > > >
> > > > A couple of thoughts and then a long suggestion:
> > > >
> > > > - Can you trim it to a smaller patchset?
> > > > It's OK if you just edit it. It does not need to reflect reality.
> > > > The reason I ask is because a ten patch set is not the example we
> > > > want to set for newbies. It's the stretch goal, not the norm.
> > >
> > > Agreed. I made the modifications and they are ready for your review.
> > Thanks
> > > >
> > > > - Your commit messages would be better if they were consistent,
> > > > either starting with upper or lower case, but not mixed.
> > > > Usually there is a pattern set in the files, but alas, when I
> > > > look at the pretty oneline of rtw_br_ext.c, there is no pattern.
> > > > (and it's messy)
> > >
> > > My apologies for the mixed case commits. I will be careful with the styles going
> > > forward. Thank you for catching that.
> > > For the purpose of proposed edits, I have edited the example per your observations.
> > >
> > Thanks
> > > >
> > > > Seque to another tutorial improvement - explaining the point
> > > > of pretty oneline, and examples would help.
> > > >
> > > > The point, is to make it easy on the eyes of the developer.
> > > > When developers scan through the history of work on a file,
> > > > it is easier to do when the appearance is consistent.
> > > >
> > > > It may not seem important to the task at hand for a newbie,
> > > > but the consistency pays off when you are staring at these
> > > > patches day in and day out.
> > > >
> > > > You'll want to alias it because you'll use it often:
> > > > alias gitpretty='git log --pretty=oneline --abbrev-commit'
> > > >
> > > > In 'Viewing your commit' it says 'You'll also want to make sure
> > > > your commit looks fine when you run these two commands:"
> > > >
> > > > Let's put some more detail around 'looks fine':
> > > > - prefix is correct
> > > > - line does not wrap on 80 Column screen
> > > > - styling matches other commits in that file with respect to use
> > > >   of punctuation, upper/lower case. Subsystems and files have
> > > >   different styles (see examples below). The point it to respect
> > > >   the style.
> > > >
> > > > This is fine:
> > > > drivers/staging/iio/adc$ gitpretty ad7816.c  (edited for fineness)
> > > >
> > > > f1b753a0f866 staging: iio: ad7816: Drop unnecessary initialization of variables
> > > > c24a4173f6bb staging: iio: ad7816: Add device tree table
> > > > 72e3a5248da9 staging: iio: ad7816: Set RD/WR pin and CONVST pin as outputs
> > > > 06c77f564ddb staging: iio: ad7816: Do not use busy_pin in case of AD7818
> > > > 073a391ca035 staging: iio: ad7816: Switch to the gpio descriptor interface
> > > >
> > > > This is *not* fine:
> > > > (line wrap, prefixes and upper/lower case are inconsistent
> > > > /staging/most/video$ gitpretty video.c  (trimmed to illustrate)
> > > >
> > > > d6ef48e59582 staging: most: video: Make use of the helper macro LIST_HEAD()
> > > > fa8db3989362 staging/most: Remove all strcpy() uses in favor of strscpy()
> > > > 6367dee9e3db staging: most: Switch from strlcpy to strscpy
> > > > b27652753918 staging: most: move core files out of the staging area
> > > > e653614ee183 media: staging/most: rename VFL_TYPE_GRABBER to _VIDEO
> > > > 08283d307444 staging: most: block module removal while having active configfs items
> > > > a20eefaee646 staging: most: Use DEFINE_SPINLOCK() instead of struct spinlock
> > > > 338d9637361c staging/most/video: set device_caps in struct video_device
> > > >
> > > > This info is not focused patchsets. Every patch that comes across this
> > > > list should make best effort to be 'gitpretty'.
> > > >
> > > > If you are out of time Deeva, perhaps percolate this up on the
> > > > list for someone else to pick up.
> > >
> > > I get your point and can relate to your suggestion based on how often I need to
> > > use the pretty command. Following suggestion will definitely make things better
> > > visually for the developers.
> > >
> > > I am thinking we should add a new sections, may be "Perform the gitpretty checks"
> > > somewhere before "submit patch" section and include the verbiage and examples
> > > (both the good and the bad example) for better understanding.
> > > Does that sound good to you?
> >
> > Yes. Deserves it's only section!
> >
> > We (esp me) in Outreachy are picky about this because newbies are
> > hopping all over staging, and by doing so, they are touching the code
> > of numerous subsystems, with different maintainers, and different
> > styles. We want to adhere to the style of the subsystem we are visiting.
>
> Hello Alison,
> I have attempted to introduce a new section and supporting information to
> accomplish guidelines on following driver style for patch work. Could you please
> and suggest if any edits are required.
>
> New section: "Following the Driver commit style"

Also requesting others, specifically the Outreachy Program participants to
review this new section and share your feedback. Your comments on making this
most useful for the participants will be helpful.

>
> Thank you,
> ./drv
>
> >
> > Thanks!
> >
> > > > ./drv >
> > > >
> > > > Thanks,
> > > > Alison
> > > > >
> > > > > >
> > > > > > Thank you,
> > > > > > ./drv
> > > > > >
> > > > > > >
> > > > > > > Thanks,
> > > > > > > Alison
> > > > > > >
> > > > > > > >
> > > > > > > > >
> > > > > > > > > Thanks,
> > > > > > > > > Alison
> > > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > >
> > > > >
> > > >
> > >
> > >
> >
>
>
>

Re: [Outreachy] Wiki updated needed - Patchset Subject

[Julia Lawall]

On Tue, 1 Nov 2022, Deepak R Varma wrote:

> On Fri, Oct 28, 2022 at 01:32:41PM -0700, Alison Schofield wrote:
> > On Sat, Oct 29, 2022 at 12:29:39AM +0530, Deepak R Varma wrote:
> > > On Thu, Oct 27, 2022 at 07:33:19PM -0700, Alison Schofield wrote:
> > > > On Fri, Oct 28, 2022 at 03:36:07AM +0530, Deepak R Varma wrote:
> > > > > On Wed, Oct 26, 2022 at 04:00:58AM +0530, Deepak Varma wrote:
> > > > > > On Mon, Oct 24, 2022 at 02:12:46PM -0700, Alison Schofield wrote:
> > > > > > > In https://kernelnewbies.org/Outreachyfirstpatch
> > > > > > >
> > > > > > > The example cover letter in 'Submitting a Patchset' has the
> > > > > > > subject "[RFC 0/7] Update REPORTING-BUGS", with no subsys.
> > > > > > > It is also confusing because the cover letter is an RFC and
> > > > > > > the patches are a mix of RFC and PATCH. Very atypical.
> > > > > > >
> > > > > > > Let's replace this example with one that was actually submitted
> > > > > > > and accepted by an Outreachy Intern.  No slight intended to the
> > > > > > > existing set as it contains an interesting set of changes. However,
> > > > > > > an example for newbies might better come from newbies.
> > > > > > >
> > > > > > > How about a patchset off this list?
> > > > > > >
> > > > > > > Would you mind using this one of yours:
> > > > > > > [PATCH v4 00/11] staging: r8188eu: trivial code cleanup patches
> > > > > > > but trim it to 3-4 patches plus the cover.
> > > > > > >
> > > > > > > It's not a trivial edit, because you need to replace all the
> > > > > > > example steps with the new patchset content.
> > > > > >
> > > > > > Hello Alison,
> > > > > > I made the changes as suggested and saved those. Looks like they might have
> > > > > > already gone to production. Sorry about that. I was thinking they will be
> > > > > > submitted to a moderator for review before releasing,
> > > > > >
> > > > > > Could you please review and let me know if any edits are required.
> > > > >
> > > > > Hello Alison,
> > > > > Did you get a chance to review the edits I made? Please share your feedback so
> > > > > that I can proceed with the next changes to the tutorial page.
> > > > >
> > > > > Thank you,
> > > > > ./drv
> > > >
> > > > Thanks for doing that! I see you were very thorough :)
> > > >
> > > > A couple of thoughts and then a long suggestion:
> > > >
> > > > - Can you trim it to a smaller patchset?
> > > > It's OK if you just edit it. It does not need to reflect reality.
> > > > The reason I ask is because a ten patch set is not the example we
> > > > want to set for newbies. It's the stretch goal, not the norm.
> > >
> > > Agreed. I made the modifications and they are ready for your review.
> > Thanks
> > > >
> > > > - Your commit messages would be better if they were consistent,
> > > > either starting with upper or lower case, but not mixed.
> > > > Usually there is a pattern set in the files, but alas, when I
> > > > look at the pretty oneline of rtw_br_ext.c, there is no pattern.
> > > > (and it's messy)
> > >
> > > My apologies for the mixed case commits. I will be careful with the styles going
> > > forward. Thank you for catching that.
> > > For the purpose of proposed edits, I have edited the example per your observations.
> > >
> > Thanks
> > > >
> > > > Seque to another tutorial improvement - explaining the point
> > > > of pretty oneline, and examples would help.
> > > >
> > > > The point, is to make it easy on the eyes of the developer.
> > > > When developers scan through the history of work on a file,
> > > > it is easier to do when the appearance is consistent.
> > > >
> > > > It may not seem important to the task at hand for a newbie,
> > > > but the consistency pays off when you are staring at these
> > > > patches day in and day out.
> > > >
> > > > You'll want to alias it because you'll use it often:
> > > > alias gitpretty='git log --pretty=oneline --abbrev-commit'
> > > >
> > > > In 'Viewing your commit' it says 'You'll also want to make sure
> > > > your commit looks fine when you run these two commands:"
> > > >
> > > > Let's put some more detail around 'looks fine':
> > > > - prefix is correct
> > > > - line does not wrap on 80 Column screen
> > > > - styling matches other commits in that file with respect to use
> > > >   of punctuation, upper/lower case. Subsystems and files have
> > > >   different styles (see examples below). The point it to respect
> > > >   the style.
> > > >
> > > > This is fine:
> > > > drivers/staging/iio/adc$ gitpretty ad7816.c  (edited for fineness)
> > > >
> > > > f1b753a0f866 staging: iio: ad7816: Drop unnecessary initialization of variables
> > > > c24a4173f6bb staging: iio: ad7816: Add device tree table
> > > > 72e3a5248da9 staging: iio: ad7816: Set RD/WR pin and CONVST pin as outputs
> > > > 06c77f564ddb staging: iio: ad7816: Do not use busy_pin in case of AD7818
> > > > 073a391ca035 staging: iio: ad7816: Switch to the gpio descriptor interface
> > > >
> > > > This is *not* fine:
> > > > (line wrap, prefixes and upper/lower case are inconsistent
> > > > /staging/most/video$ gitpretty video.c  (trimmed to illustrate)
> > > >
> > > > d6ef48e59582 staging: most: video: Make use of the helper macro LIST_HEAD()
> > > > fa8db3989362 staging/most: Remove all strcpy() uses in favor of strscpy()
> > > > 6367dee9e3db staging: most: Switch from strlcpy to strscpy
> > > > b27652753918 staging: most: move core files out of the staging area
> > > > e653614ee183 media: staging/most: rename VFL_TYPE_GRABBER to _VIDEO
> > > > 08283d307444 staging: most: block module removal while having active configfs items
> > > > a20eefaee646 staging: most: Use DEFINE_SPINLOCK() instead of struct spinlock
> > > > 338d9637361c staging/most/video: set device_caps in struct video_device
> > > >
> > > > This info is not focused patchsets. Every patch that comes across this
> > > > list should make best effort to be 'gitpretty'.
> > > >
> > > > If you are out of time Deeva, perhaps percolate this up on the
> > > > list for someone else to pick up.
> > >
> > > I get your point and can relate to your suggestion based on how often I need to
> > > use the pretty command. Following suggestion will definitely make things better
> > > visually for the developers.
> > >
> > > I am thinking we should add a new sections, may be "Perform the gitpretty checks"
> > > somewhere before "submit patch" section and include the verbiage and examples
> > > (both the good and the bad example) for better understanding.
> > > Does that sound good to you?
> >
> > Yes. Deserves it's only section!
> >
> > We (esp me) in Outreachy are picky about this because newbies are
> > hopping all over staging, and by doing so, they are touching the code
> > of numerous subsystems, with different maintainers, and different
> > styles. We want to adhere to the style of the subsystem we are visiting.
>
> Hello Alison,
> I have attempted to introduce a new section and supporting information to
> accomplish guidelines on following driver style for patch work. Could you please
> and suggest if any edits are required.
>
> New section: "Following the Driver commit style"

reduces confusion/errors and easy on eyes. ->
reduces confusion/errors and is easy on eyes.

Use `git log <<file.c>>` command to review ->
Use the `git log <<file.c>>` command to review

patch log message -> the patch log message

Later in the same sentence, I odn't know what you ean by credentials.

Additionally, following git command provides ->
Additionally, the following git command provides

The output for:

git log --pretty=oneline --abbrev-commit

looks the same as

git log --oneline

The latter should already be suggested somewhere in the documentation.
Maybe this place is better, and it should be removed from wher it was
before?

"Since this is one of the most frequently used git commands, you should
add this command to your shell profile using the following bashrc alias:"
"you should" seems excessive.  git log --oneline is pretty easy to type
already.

Use consistent patch prefix as available from the history ->
Use a consistent patch prefix as available from the history

small case -> lower case

capitalized case -> upper case

Put best effort to keep -> Try to keep

The example of inconsistent commits is clearly not great, but one can end
up with inconsistent commits when some patches cover only one file and
some patches cover multiple files.

julia

Re: [Outreachy] Wiki updated needed - Patchset Subject

[Alison Schofield]

On Mon, Oct 31, 2022 at 10:33:10PM +0100, Julia Lawall wrote:
> 
> 
> On Tue, 1 Nov 2022, Deepak R Varma wrote:
> 
> > On Fri, Oct 28, 2022 at 01:32:41PM -0700, Alison Schofield wrote:
> > > On Sat, Oct 29, 2022 at 12:29:39AM +0530, Deepak R Varma wrote:
> > > > On Thu, Oct 27, 2022 at 07:33:19PM -0700, Alison Schofield wrote:
> > > > > On Fri, Oct 28, 2022 at 03:36:07AM +0530, Deepak R Varma wrote:
> > > > > > On Wed, Oct 26, 2022 at 04:00:58AM +0530, Deepak Varma wrote:
> > > > > > > On Mon, Oct 24, 2022 at 02:12:46PM -0700, Alison Schofield wrote:
> > > > > > > > In https://kernelnewbies.org/Outreachyfirstpatch
> > > > > > > >
> > > > > > > > The example cover letter in 'Submitting a Patchset' has the
> > > > > > > > subject "[RFC 0/7] Update REPORTING-BUGS", with no subsys.
> > > > > > > > It is also confusing because the cover letter is an RFC and
> > > > > > > > the patches are a mix of RFC and PATCH. Very atypical.
> > > > > > > >
> > > > > > > > Let's replace this example with one that was actually submitted
> > > > > > > > and accepted by an Outreachy Intern.  No slight intended to the
> > > > > > > > existing set as it contains an interesting set of changes. However,
> > > > > > > > an example for newbies might better come from newbies.
> > > > > > > >
> > > > > > > > How about a patchset off this list?
> > > > > > > >
> > > > > > > > Would you mind using this one of yours:
> > > > > > > > [PATCH v4 00/11] staging: r8188eu: trivial code cleanup patches
> > > > > > > > but trim it to 3-4 patches plus the cover.
> > > > > > > >
> > > > > > > > It's not a trivial edit, because you need to replace all the
> > > > > > > > example steps with the new patchset content.
> > > > > > >
> > > > > > > Hello Alison,
> > > > > > > I made the changes as suggested and saved those. Looks like they might have
> > > > > > > already gone to production. Sorry about that. I was thinking they will be
> > > > > > > submitted to a moderator for review before releasing,
> > > > > > >
> > > > > > > Could you please review and let me know if any edits are required.
> > > > > >
> > > > > > Hello Alison,
> > > > > > Did you get a chance to review the edits I made? Please share your feedback so
> > > > > > that I can proceed with the next changes to the tutorial page.
> > > > > >
> > > > > > Thank you,
> > > > > > ./drv
> > > > >
> > > > > Thanks for doing that! I see you were very thorough :)
> > > > >
> > > > > A couple of thoughts and then a long suggestion:
> > > > >
> > > > > - Can you trim it to a smaller patchset?
> > > > > It's OK if you just edit it. It does not need to reflect reality.
> > > > > The reason I ask is because a ten patch set is not the example we
> > > > > want to set for newbies. It's the stretch goal, not the norm.
> > > >
> > > > Agreed. I made the modifications and they are ready for your review.
> > > Thanks
> > > > >
> > > > > - Your commit messages would be better if they were consistent,
> > > > > either starting with upper or lower case, but not mixed.
> > > > > Usually there is a pattern set in the files, but alas, when I
> > > > > look at the pretty oneline of rtw_br_ext.c, there is no pattern.
> > > > > (and it's messy)
> > > >
> > > > My apologies for the mixed case commits. I will be careful with the styles going
> > > > forward. Thank you for catching that.
> > > > For the purpose of proposed edits, I have edited the example per your observations.
> > > >
> > > Thanks
> > > > >
> > > > > Seque to another tutorial improvement - explaining the point
> > > > > of pretty oneline, and examples would help.
> > > > >
> > > > > The point, is to make it easy on the eyes of the developer.
> > > > > When developers scan through the history of work on a file,
> > > > > it is easier to do when the appearance is consistent.
> > > > >
> > > > > It may not seem important to the task at hand for a newbie,
> > > > > but the consistency pays off when you are staring at these
> > > > > patches day in and day out.
> > > > >
> > > > > You'll want to alias it because you'll use it often:
> > > > > alias gitpretty='git log --pretty=oneline --abbrev-commit'
> > > > >
> > > > > In 'Viewing your commit' it says 'You'll also want to make sure
> > > > > your commit looks fine when you run these two commands:"
> > > > >
> > > > > Let's put some more detail around 'looks fine':
> > > > > - prefix is correct
> > > > > - line does not wrap on 80 Column screen
> > > > > - styling matches other commits in that file with respect to use
> > > > >   of punctuation, upper/lower case. Subsystems and files have
> > > > >   different styles (see examples below). The point it to respect
> > > > >   the style.
> > > > >
> > > > > This is fine:
> > > > > drivers/staging/iio/adc$ gitpretty ad7816.c  (edited for fineness)
> > > > >
> > > > > f1b753a0f866 staging: iio: ad7816: Drop unnecessary initialization of variables
> > > > > c24a4173f6bb staging: iio: ad7816: Add device tree table
> > > > > 72e3a5248da9 staging: iio: ad7816: Set RD/WR pin and CONVST pin as outputs
> > > > > 06c77f564ddb staging: iio: ad7816: Do not use busy_pin in case of AD7818
> > > > > 073a391ca035 staging: iio: ad7816: Switch to the gpio descriptor interface
> > > > >
> > > > > This is *not* fine:
> > > > > (line wrap, prefixes and upper/lower case are inconsistent
> > > > > /staging/most/video$ gitpretty video.c  (trimmed to illustrate)
> > > > >
> > > > > d6ef48e59582 staging: most: video: Make use of the helper macro LIST_HEAD()
> > > > > fa8db3989362 staging/most: Remove all strcpy() uses in favor of strscpy()
> > > > > 6367dee9e3db staging: most: Switch from strlcpy to strscpy
> > > > > b27652753918 staging: most: move core files out of the staging area
> > > > > e653614ee183 media: staging/most: rename VFL_TYPE_GRABBER to _VIDEO
> > > > > 08283d307444 staging: most: block module removal while having active configfs items
> > > > > a20eefaee646 staging: most: Use DEFINE_SPINLOCK() instead of struct spinlock
> > > > > 338d9637361c staging/most/video: set device_caps in struct video_device
> > > > >
> > > > > This info is not focused patchsets. Every patch that comes across this
> > > > > list should make best effort to be 'gitpretty'.
> > > > >
> > > > > If you are out of time Deeva, perhaps percolate this up on the
> > > > > list for someone else to pick up.
> > > >
> > > > I get your point and can relate to your suggestion based on how often I need to
> > > > use the pretty command. Following suggestion will definitely make things better
> > > > visually for the developers.
> > > >
> > > > I am thinking we should add a new sections, may be "Perform the gitpretty checks"
> > > > somewhere before "submit patch" section and include the verbiage and examples
> > > > (both the good and the bad example) for better understanding.
> > > > Does that sound good to you?
> > >
> > > Yes. Deserves it's only section!
> > >
> > > We (esp me) in Outreachy are picky about this because newbies are
> > > hopping all over staging, and by doing so, they are touching the code
> > > of numerous subsystems, with different maintainers, and different
> > > styles. We want to adhere to the style of the subsystem we are visiting.
> >
> > Hello Alison,
> > I have attempted to introduce a new section and supporting information to
> > accomplish guidelines on following driver style for patch work. Could you please
> > and suggest if any edits are required.
> >
> > New section: "Following the Driver commit style"
> 
> reduces confusion/errors and easy on eyes. ->
> reduces confusion/errors and is easy on eyes.
> 
> Use `git log <<file.c>>` command to review ->
> Use the `git log <<file.c>>` command to review
> 
> patch log message -> the patch log message
> 
> Later in the same sentence, I odn't know what you ean by credentials.
> 
> Additionally, following git command provides ->
> Additionally, the following git command provides
> 
> The output for:
> 
> git log --pretty=oneline --abbrev-commit
> 
> looks the same as
> 
> git log --oneline
> 
> The latter should already be suggested somewhere in the documentation.
> Maybe this place is better, and it should be removed from wher it was
> before?
> 
> "Since this is one of the most frequently used git commands, you should
> add this command to your shell profile using the following bashrc alias:"
> "you should" seems excessive.  git log --oneline is pretty easy to type
> already.

Deeva,
I picked this up "git log --pretty=oneline --abbrev-commit" from
Outreachy in 2016 and have been using it as my 'gitpretty' ever
since :)  

You'll find it in the section 'Exploring the kernel tree'.
Change that to be the simpler "git log --oneline"

That's why we need fresh eyes to update the tutorial!!!

> 
> Use consistent patch prefix as available from the history ->
> Use a consistent patch prefix as available from the history
> 

I may have missed this...

Use git log --oneline, in an 80 column view, to confirm the commit message
does not wrap the line.

> small case -> lower case
> 
> capitalized case -> upper case
> 
> Put best effort to keep -> Try to keep
> 
> The example of inconsistent commits is clearly not great, but one can end
> up with inconsistent commits when some patches cover only one file and
> some patches cover multiple files.
> 
> julia
> 

Re: [Outreachy] Wiki updated needed - Patchset Subject

[Deepak R Varma]

On Mon, Oct 31, 2022 at 06:39:23PM -0700, Alison Schofield wrote:
> On Mon, Oct 31, 2022 at 10:33:10PM +0100, Julia Lawall wrote:
> >
> >
> > On Tue, 1 Nov 2022, Deepak R Varma wrote:
> >
> > > On Fri, Oct 28, 2022 at 01:32:41PM -0700, Alison Schofield wrote:
> > > > On Sat, Oct 29, 2022 at 12:29:39AM +0530, Deepak R Varma wrote:
> > >
> > > Hello Alison,
> > > I have attempted to introduce a new section and supporting information to
> > > accomplish guidelines on following driver style for patch work. Could you please
> > > and suggest if any edits are required.
> > >
> > > New section: "Following the Driver commit style"
> >
> > reduces confusion/errors and easy on eyes. ->
> > reduces confusion/errors and is easy on eyes.
> >
> > Use `git log <<file.c>>` command to review ->
> > Use the `git log <<file.c>>` command to review
> >
> > patch log message -> the patch log message
> >
> > Later in the same sentence, I odn't know what you ean by credentials.
> >
> > Additionally, following git command provides ->
> > Additionally, the following git command provides
> >
> > The output for:
> >
> > git log --pretty=oneline --abbrev-commit
> >
> > looks the same as
> >
> > git log --oneline
> >
> > The latter should already be suggested somewhere in the documentation.
> > Maybe this place is better, and it should be removed from wher it was
> > before?
> >
> > "Since this is one of the most frequently used git commands, you should
> > add this command to your shell profile using the following bashrc alias:"
> > "you should" seems excessive.  git log --oneline is pretty easy to type
> > already.
>
> Deeva,
> I picked this up "git log --pretty=oneline --abbrev-commit" from
> Outreachy in 2016 and have been using it as my 'gitpretty' ever
> since :)
>
> You'll find it in the section 'Exploring the kernel tree'.
> Change that to be the simpler "git log --oneline"
>
> That's why we need fresh eyes to update the tutorial!!!
>
> >
> > Use consistent patch prefix as available from the history ->
> > Use a consistent patch prefix as available from the history
> >
>
> I may have missed this...
>
> Use git log --oneline, in an 80 column view, to confirm the commit message
> does not wrap the line.
>
> > small case -> lower case
> >
> > capitalized case -> upper case
> >
> > Put best effort to keep -> Try to keep
> >
> > The example of inconsistent commits is clearly not great, but one can end
> > up with inconsistent commits when some patches cover only one file and
> > some patches cover multiple files.
> >
> > julia

Thank you very much Julia and Alison for the review and feedback. I have updated
the page accordingly and can be reviewed. Let me know if you have any further
comments.

> >
>

Re: [Outreachy] Wiki updated needed - Patchset Subject

[Alison Schofield]

On Tue, Nov 01, 2022 at 10:31:53AM +0530, Deepak R Varma wrote:
> On Mon, Oct 31, 2022 at 06:39:23PM -0700, Alison Schofield wrote:
> > On Mon, Oct 31, 2022 at 10:33:10PM +0100, Julia Lawall wrote:
> > >
> > >
> > > On Tue, 1 Nov 2022, Deepak R Varma wrote:
> > >
> > > > On Fri, Oct 28, 2022 at 01:32:41PM -0700, Alison Schofield wrote:
> > > > > On Sat, Oct 29, 2022 at 12:29:39AM +0530, Deepak R Varma wrote:
> > > >
> > > > Hello Alison,
> > > > I have attempted to introduce a new section and supporting information to
> > > > accomplish guidelines on following driver style for patch work. Could you please
> > > > and suggest if any edits are required.
> > > >
> > > > New section: "Following the Driver commit style"
> > >
> > > reduces confusion/errors and easy on eyes. ->
> > > reduces confusion/errors and is easy on eyes.
> > >
> > > Use `git log <<file.c>>` command to review ->
> > > Use the `git log <<file.c>>` command to review
> > >
> > > patch log message -> the patch log message
> > >
> > > Later in the same sentence, I odn't know what you ean by credentials.
> > >
> > > Additionally, following git command provides ->
> > > Additionally, the following git command provides
> > >
> > > The output for:
> > >
> > > git log --pretty=oneline --abbrev-commit
> > >
> > > looks the same as
> > >
> > > git log --oneline
> > >
> > > The latter should already be suggested somewhere in the documentation.
> > > Maybe this place is better, and it should be removed from wher it was
> > > before?
> > >
> > > "Since this is one of the most frequently used git commands, you should
> > > add this command to your shell profile using the following bashrc alias:"
> > > "you should" seems excessive.  git log --oneline is pretty easy to type
> > > already.
> >
> > Deeva,
> > I picked this up "git log --pretty=oneline --abbrev-commit" from
> > Outreachy in 2016 and have been using it as my 'gitpretty' ever
> > since :)
> >
> > You'll find it in the section 'Exploring the kernel tree'.
> > Change that to be the simpler "git log --oneline"
> >
> > That's why we need fresh eyes to update the tutorial!!!
> >
> > >
> > > Use consistent patch prefix as available from the history ->
> > > Use a consistent patch prefix as available from the history
> > >
> >
> > I may have missed this...
> >
> > Use git log --oneline, in an 80 column view, to confirm the commit message
> > does not wrap the line.
> >
> > > small case -> lower case
> > >
> > > capitalized case -> upper case
> > >
> > > Put best effort to keep -> Try to keep
> > >
> > > The example of inconsistent commits is clearly not great, but one can end
> > > up with inconsistent commits when some patches cover only one file and
> > > some patches cover multiple files.
> > >
> > > julia
> 
> Thank you very much Julia and Alison for the review and feedback. I have updated
> the page accordingly and can be reviewed. Let me know if you have any further
> comments.

Thanks Deeva! I looked it over and it looks very thorough and useful.
Alison

> 
> > >
> >
> 
> 
>