All of lore.kernel.org
 help / color / mirror / Atom feed
* TODO/comment questions
@ 2016-09-27 19:49 gnudevliz
  2016-09-27 20:04 ` [Outreachy kernel] " Alison Schofield
       [not found] ` <20160927203553.GA20780@d830.WORKGROUP>
  0 siblings, 2 replies; 6+ messages in thread
From: gnudevliz @ 2016-09-27 19:49 UTC (permalink / raw)
  To: outreachy-kernel


[-- Attachment #1.1: Type: text/plain, Size: 580 bytes --]

I'm seeing a "Block comments use * on subsequent lines" checkpatch error in 
many spots on one driver so I thought I would change a few of those in 
multiple files and put them all in one patch. I think that's better than 
submitting multiple patches just for comment revision, but you tell me.

My second question-- one specific comment that is missing an asterisk 
includes a TODO line. Is it good practice to have a TODO in a comment? I'm 
not seeing anything about it in the style guide. If it's fine I'll just add 
an asterisk there and leave it the way it is. 

Thanks,
Liz

[-- Attachment #1.2: Type: text/html, Size: 683 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Outreachy kernel] TODO/comment questions
  2016-09-27 19:49 TODO/comment questions gnudevliz
@ 2016-09-27 20:04 ` Alison Schofield
  2016-09-27 20:16   ` Julia Lawall
       [not found] ` <20160927203553.GA20780@d830.WORKGROUP>
  1 sibling, 1 reply; 6+ messages in thread
From: Alison Schofield @ 2016-09-27 20:04 UTC (permalink / raw)
  To: gnudevliz; +Cc: outreachy-kernel

On Tue, Sep 27, 2016 at 12:49:37PM -0700, gnudevliz@gmail.com wrote:
> I'm seeing a "Block comments use * on subsequent lines" checkpatch error in 
> many spots on one driver so I thought I would change a few of those in 
> multiple files and put them all in one patch. I think that's better than 
> submitting multiple patches just for comment revision, but you tell me.

Sounds good.
Single driver with multiple files needing same type of change.
So the commit msg applies to everything you did..ie use kernel preferred block
commenting style.

> 
> My second question-- one specific comment that is missing an asterisk 
> includes a TODO line. Is it good practice to have a TODO in a comment? I'm 
> not seeing anything about it in the style guide. If it's fine I'll just add 
> an asterisk there and leave it the way it is. 

When I "git grep TODO", I see the TODO inside comments a lot.
Don't know if it's good practice, but certainly common.
What would the alternative be?

alisons
> 
> Thanks,
> Liz
> 
> -- 
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/44f080fb-8ee2-4de0-8480-7b21534757ab%40googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Outreachy kernel] TODO/comment questions
  2016-09-27 20:04 ` [Outreachy kernel] " Alison Schofield
@ 2016-09-27 20:16   ` Julia Lawall
  0 siblings, 0 replies; 6+ messages in thread
From: Julia Lawall @ 2016-09-27 20:16 UTC (permalink / raw)
  To: Alison Schofield; +Cc: gnudevliz, outreachy-kernel

> > My second question-- one specific comment that is missing an asterisk
> > includes a TODO line. Is it good practice to have a TODO in a comment? I'm
> > not seeing anything about it in the style guide. If it's fine I'll just add
> > an asterisk there and leave it the way it is.
>
> When I "git grep TODO", I see the TODO inside comments a lot.
> Don't know if it's good practice, but certainly common.
> What would the alternative be?

If the developer had something more elaborate in mind, it would indeed
have been nice to have it expressed in a more detailed way.  But at least
it conveys the information that there is something to do here...

julia


^ permalink raw reply	[flat|nested] 6+ messages in thread

* line wrap and rebase question
       [not found]     ` <20160927222001.GA21768@d830.WORKGROUP>
@ 2016-09-28 18:19       ` Elizabeth Ferdman
  2016-09-28 20:32         ` Alison Schofield
  2016-09-28 20:43         ` [Outreachy kernel] " Julia Lawall
  0 siblings, 2 replies; 6+ messages in thread
From: Elizabeth Ferdman @ 2016-09-28 18:19 UTC (permalink / raw)
  To: Alison Schofield; +Cc: outreachy-kernel

Hey Alison,

Good morning! I received some not so good feedback from Greg that I want
to fix and I have a bunch of questions. The plus side is that maybe
some of the answers could go into the tutorial.

First he said "please wrap your changelog comments at 72 columns."

Is this something I add to my .vimrc? I've been using "git commit -m".
is that causing the issue? I thought git commit messages were supposed
to be less than 50 chars anyway. I'm confused. 

The other issue was that my commit didn't apply to his tree. He says
please rebase and resend. I think I need to do:

$ git fetch origin
$ git rebase origin/staging-testing
Then reformat the patch?
$ git format-patch -o ~/mypatches/ HEAD^
$ mutt -H <file>

So I don't need to uncommit/recommit?

Thanks,
Liz


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: line wrap and rebase question
  2016-09-28 18:19       ` line wrap and rebase question Elizabeth Ferdman
@ 2016-09-28 20:32         ` Alison Schofield
  2016-09-28 20:43         ` [Outreachy kernel] " Julia Lawall
  1 sibling, 0 replies; 6+ messages in thread
From: Alison Schofield @ 2016-09-28 20:32 UTC (permalink / raw)
  To: Elizabeth Ferdman; +Cc: outreachy-kernel

reply is in your new thread - Rebase/line wrap question


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Outreachy kernel] line wrap and rebase question
  2016-09-28 18:19       ` line wrap and rebase question Elizabeth Ferdman
  2016-09-28 20:32         ` Alison Schofield
@ 2016-09-28 20:43         ` Julia Lawall
  1 sibling, 0 replies; 6+ messages in thread
From: Julia Lawall @ 2016-09-28 20:43 UTC (permalink / raw)
  To: Elizabeth Ferdman; +Cc: Alison Schofield, outreachy-kernel



On Wed, 28 Sep 2016, Elizabeth Ferdman wrote:

> Hey Alison,
>
> Good morning! I received some not so good feedback from Greg that I want
> to fix and I have a bunch of questions. The plus side is that maybe
> some of the answers could go into the tutorial.
>
> First he said "please wrap your changelog comments at 72 columns."
>
> Is this something I add to my .vimrc? I've been using "git commit -m".
> is that causing the issue? I thought git commit messages were supposed
> to be less than 50 chars anyway. I'm confused.

The 50 chars is for the subject line.  The 72 chars is for the log
message.  Once the patch has been committed, the log message will be
indented a bit.  So you can't go all the way to 80 characters.  For code,
going a bit past 72 can be OK.

julia

>
> The other issue was that my commit didn't apply to his tree. He says
> please rebase and resend. I think I need to do:
>
> $ git fetch origin
> $ git rebase origin/staging-testing
> Then reformat the patch?
> $ git format-patch -o ~/mypatches/ HEAD^
> $ mutt -H <file>
>
> So I don't need to uncommit/recommit?
>
> Thanks,
> Liz
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20160928181956.GA2193%40localhost.
> For more options, visit https://groups.google.com/d/optout.
>


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-09-28 20:43 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-27 19:49 TODO/comment questions gnudevliz
2016-09-27 20:04 ` [Outreachy kernel] " Alison Schofield
2016-09-27 20:16   ` Julia Lawall
     [not found] ` <20160927203553.GA20780@d830.WORKGROUP>
     [not found]   ` <20160927220413.GA23877@localhost>
     [not found]     ` <20160927222001.GA21768@d830.WORKGROUP>
2016-09-28 18:19       ` line wrap and rebase question Elizabeth Ferdman
2016-09-28 20:32         ` Alison Schofield
2016-09-28 20:43         ` [Outreachy kernel] " Julia Lawall

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.