[Buildroot] commiting patches

[Buildroot] commiting patches

[Heiko Zuerker]

Hey,

I was wondering for a while, what are the criterias for patches to be  
applied to buildroot.
Some of them seem to get applied right away, others were sent in a  
month or two ago and don't. I understand that some probably will never  
get applied, but will there be a "sorry dude, but..." email?

Just curious how this is handled.

-- 

Regards
   Heiko Zuerker
   http://www.devil-linux.org


----------------------------------------------------------------
This message was sent using IMP, the Internet Messaging Program.

[Buildroot] commiting patches

[Yann E. MORIN]

Heiko, All,

On Thursday 30 December 2010 13:30:05 Heiko Zuerker wrote:
> I was wondering for a while, what are the criterias for patches to be  
> applied to buildroot.
> Some of them seem to get applied right away, others were sent in a  
> month or two ago and don't. I understand that some probably will never  
> get applied, but will there be a "sorry dude, but..." email?
> Just curious how this is handled.

Well, I won't speak for Peter, or any other participant, but here's what
I believe is the proper way to act:

0) prefer sending:
   - new features asap after a release
   - bug fixes: always
   - in a patch series, bug fixes come first, features come last
     - that way, even if your new feature has issues, or is not
       interesting, at least your bug fixes can be applied
   - in a patch series, trivial changes go first, and more complex
     ones go to the end, as much as possible, so that you get a bit
     more chance to get part of the series applied
   - once a series as been partiually applied (cause of bug fixes
     or new feature), you get a better chance that the remaining in
     the next iteration gets more attention
   - MOST IMPORTANT: do not expect people to be too much reactive
     during holidays season, such as X-Mas, when people tend to be
     with their families, and might not have that much time to
     dedicate to FLOSS projects. ;-)
   - look at who is usually changing the files you touch, and CC them
     as that will make them notice. Do not CC too many people, or
     everyone might think the series will be handled by someone esle
     (max 2 CC is good fo BR, I believe)

1) send your patch series:
   - properly document each patch
   - have an explicit commit message, with first line as descriptive
     as you can (80-char wide)
   - do not forget to SoB each patch
   - for long and/or complex patch series (even single patch), write
     an introductory message that is not gonna be part of the commit
     message, where you can explain at large what the series does,
     and explain the reasons for the changes (eg. design, bigger plan
     long-term...)
   - if it makes sense, split the patch series into shorter series,
     as that gets easier to review; say the second series depends on
     the first one. Even wait for the first to get applied before
     sending the next one

2) three cases
   a) some patches in the series get applied: end of story for those! :-)
   b) someone answers with comments
      - adress the comments
        - explain your position
        - or accept to change/fix the affected patch(es)
      - wait a litle bit, in case someone else chimes in to comment
      - update the series, and go back to 1)
   c) nobody answers
      - wait a litle bit (about three/four days)
      - answer the top-level mesage with something like:
          Ping? Any comment on that series/patch?
          Cheers! ;-)

3) if after that noone answers
   - wait a little longer, such as a good 10-15 days after your ping
   - for features, even wait for after the next release if it gets too
     close
     - better not flood the maintainer just before a release, you may
       get better chance to be noticed just after the release
   - repost the whole series
   - say that it is a repost, and why you think it is important

4) if after that, noone answers, then end of story.

That applies to about all FLOSS projects, I believe! ;-)

Patch series getting no comment might simply be because the maintainer
missed it, or forget about it (hence the ping above). Of course, rejected
series should get notified with the reasons for the rejection. :-/

But usually, a ping is enough to get noticed. :-)

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] commiting patches

[Peter Korsgaard]

>>>>> "Yann" == Yann E MORIN <yann.morin.1998@anciens.enib.fr> writes:

Hi,

 >> I was wondering for a while, what are the criterias for patches to
 >> be applied to buildroot.  Some of them seem to get applied right
 >> away, others were sent in a month or two ago and don't. I understand
 >> that some probably will never get applied, but will there be a
 >> "sorry dude, but..." email?  Just curious how this is handled.

 Yann> Well, I won't speak for Peter, or any other participant, but
 Yann> here's what I believe is the proper way to act:

Thanks for this extended guide! I do completely agree with it. Minor
issues such as style / format DO matter for what patches I look at
first.

-- 
Bye, Peter Korsgaard

[Buildroot] commiting patches

[Peter Korsgaard]

>>>>> "Heiko" == Heiko Zuerker <heiko@zuerker.org> writes:

 Heiko> Hey,

 Heiko> I was wondering for a while, what are the criterias for patches
 Heiko> to be applied to buildroot.
 Heiko> Some of them seem to get applied right away, others were sent in a
 Heiko> month or two ago and don't. I understand that some probably will never
 Heiko> get applied, but will there be a "sorry dude, but..." email?

I'm not following any strict procedure, but I do try to sooner or later
get around to review all patches. Understand that I do this in my spare
time, so turnaround time can sometimes be slow.

I mainly try to handle the patches in fifo mode, but I do also skim new
mails. This is both because sometimes updates are sent, and sometimes I
see trivial patches that can be handled when I have 5 min spare or
important bugfixes.

It's a big help when other developers help with the review and send
their ack/nacks (thanks for this!), so those patches are often handled
earlier than others.

But I can still forget about patches, so if you haven't had any feedback
for a week or two, please check if it still applies and send a followup
mail.

It might be politically incorrect, but the patch author also does
matter. If the patch is from a regular contributor who normally does
good work, then I know I don't need to review in as much details as if
it's from a complete stranger - So a good way of getting your own
patches applied faster is to help reviewing other patches ;)

-- 
Bye, Peter Korsgaard

[Buildroot] commiting patches

[Daniel Nyström]

2010/12/30 Peter Korsgaard <jacmet@uclibc.org>:
> It's a big help when other developers help with the review and send
> their ack/nacks (thanks for this!), so those patches are often handled
> earlier than others.

How do you prefer the ack/nack? Just a reply with "+1" in mail body? A
"Signed-off-by: name <mail>" mail body? or the whole patch resubmitted
with the SoB-line added?

[Buildroot] commiting patches

[Sam Ravnborg]

On Thu, Dec 30, 2010 at 10:04:13PM +0100, Daniel Nystr?m wrote:
> 2010/12/30 Peter Korsgaard <jacmet@uclibc.org>:
> > It's a big help when other developers help with the review and send
> > their ack/nacks (thanks for this!), so those patches are often handled
> > earlier than others.
> 
> How do you prefer the ack/nack? Just a reply with "+1" in mail body? A
> "Signed-off-by: name <mail>" mail body? or the whole patch resubmitted
> with the SoB-line added?

On linux-kernel the rules are mosre or less:

You provide an ack by including the fomal ack line in a mail like this:
Acked-by: Sam Ravnborg <sam@ravnborg.org>

This makes it easy for the submitter to copy the line and
you have it using your preferred spelling / mail address.
There are people that does not use their regular mail address
the acked-by lines.
Sometimes submittes interprets mails like "+1" as Acked-by: - sometimes not.


You use a sob line only if you are in the chain that _handle_ a patch.
So you can follow the path of the patch by following the sob-lines.

Lets consider a simple example.
I create a patch for which I add my:
Signed-off-by: Sam Ravnborg <sam@ravnborg.org>

Then you pick it up - and potentially modifying it.
To document that you are in the chain handling the
patch you add your SOB line below mine.
So it would now look like this:

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Daniel Nystr?m <daniel.nystrom@timeterminal.se>

And then Peter picks it up and when he applies it he add his SOB line as well.


So the applied patch will look like this:

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Daniel Nystr?m <daniel.nystrom@timeterminal.se>
Signed-off-by: Peter Korsgaard <jacmet@uclibc.org>


	Sam

[Buildroot] commiting patches

[Peter Korsgaard]

>>>>> "Daniel" == Daniel Nystr?m <daniel.nystrom@timeterminal.se> writes:

 Daniel> 2010/12/30 Peter Korsgaard <jacmet@uclibc.org>:
 >> It's a big help when other developers help with the review and send
 >> their ack/nacks (thanks for this!), so those patches are often handled
 >> earlier than others.

 Daniel> How do you prefer the ack/nack? Just a reply with "+1" in mail
 Daniel> body? A "Signed-off-by: name <mail>" mail body? or the whole
 Daniel> patch resubmitted with the SoB-line added?

I prefer an Acked-by: tag like it's done for the Linux kernel and a
bunch of other projects using git (and like Thomas already does).

-- 
Bye, Peter Korsgaard

[Buildroot] commiting patches

[Daniel Nyström]

Den 30 december 2010 23:07 skrev Peter Korsgaard <jacmet@uclibc.org>:
> I prefer an Acked-by: tag like it's done for the Linux kernel and a
> bunch of other projects using git (and like Thomas already does).

Just a one-line mail with "Acked-by: Me <mymail>"?  (sorry for being picky)

[Buildroot] commiting patches

[Peter Korsgaard]

>>>>> "Daniel" == Daniel Nystr?m <daniel.nystrom@timeterminal.se> writes:

 Daniel> Den 30 december 2010 23:07 skrev Peter Korsgaard <jacmet@uclibc.org>:
 >> I prefer an Acked-by: tag like it's done for the Linux kernel and a
 >> bunch of other projects using git (and like Thomas already does).

 Daniel> Just a one-line mail with "Acked-by: Me <mymail>"?  (sorry for being picky)

Yes - And any additional comments you might have.

-- 
Bye, Peter Korsgaard

[Buildroot] commiting patches

[Heiko Zuerker]

Quoting Peter Korsgaard <jacmet@uclibc.org>:

>>>>>> "Daniel" == Daniel Nystr?m <daniel.nystrom@timeterminal.se> writes:
>
>  Daniel> 2010/12/30 Peter Korsgaard <jacmet@uclibc.org>:
>  >> It's a big help when other developers help with the review and send
>  >> their ack/nacks (thanks for this!), so those patches are often handled
>  >> earlier than others.
>
>  Daniel> How do you prefer the ack/nack? Just a reply with "+1" in mail
>  Daniel> body? A "Signed-off-by: name <mail>" mail body? or the whole
>  Daniel> patch resubmitted with the SoB-line added?
>
> I prefer an Acked-by: tag like it's done for the Linux kernel and a
> bunch of other projects using git (and like Thomas already does).

This may be a stupid question: you want the Acked-by in the patch file  
itself, right?
Is there an easy way to do this if there are multiple patches?

-- 

Regards
   Heiko Zuerker
   http://www.devil-linux.org


----------------------------------------------------------------
This message was sent using IMP, the Internet Messaging Program.

[Buildroot] commiting patches

[Peter Korsgaard]

>>>>> "Heiko" == Heiko Zuerker <heiko@zuerker.org> writes:

Hi,

 >> I prefer an Acked-by: tag like it's done for the Linux kernel and a
 >> bunch of other projects using git (and like Thomas already does).

 Heiko> This may be a stupid question: you want the Acked-by in the patch file
 Heiko> itself, right?
 Heiko> Is there an easy way to do this if there are multiple patches?

Well, by nature of a review you have to look at each mail, so just hit
reply and add your acked-by (or script your editor).

-- 
Bye, Peter Korsgaard

[Buildroot] commiting patches

[Bryan Hundven]

On Thu, Dec 30, 2010 at 2:30 PM, Peter Korsgaard <jacmet@uclibc.org> wrote:
>>>>>> "Heiko" == Heiko Zuerker <heiko@zuerker.org> writes:
>
> Hi,
>
> ?>> I prefer an Acked-by: tag like it's done for the Linux kernel and a
> ?>> bunch of other projects using git (and like Thomas already does).
>
> ?Heiko> This may be a stupid question: you want the Acked-by in the patch file
> ?Heiko> itself, right?
> ?Heiko> Is there an easy way to do this if there are multiple patches?
>
> Well, by nature of a review you have to look at each mail, so just hit
> reply and add your acked-by (or script your editor).

No editor/plugin war intended.

I use snipMate: http://www.vim.org/scripts/script.php?script_id=2540

a vim plugin. I add the following to ~/.vim/snippets/_.snippets

-------------8<-----------------8<------------------
snippet ack
    Acked-by: ${1}
snippet rev
    Reviewed-by: ${1}
snippet sob
    Signed-off-by: ${1}
snippet tested
    Tested-by: ${1}
snippet me
    Bryan Hundven <my@email.address>
-------------8<-----------------8<------------------

Then when I'm in vim looking@a patch or a change, I just type:

ack<TAB>me<TAB>

or

sob<TAB>me<TAB>

I'm sure this is possible in other editors as well. None is better
then the other; just how I personally handle this stuff.

Hope this is helpful...

> --
> Bye, Peter Korsgaard
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
>

-Bryan

P.S. I got this idea from Peter Hutterer:
http://who-t.blogspot.com/2010/11/adding-reviewed-by-tags-to-patches.html
He originally had this in ~/.vim/snippets/gitcommit.snippets, but
since I work with more then just git, I added it to mine to the global
snippets.

[Buildroot] commiting patches

[Heiko Zuerker]

Quoting Peter Korsgaard <jacmet@uclibc.org>:
>>>>>> "Heiko" == Heiko Zuerker <heiko@zuerker.org> writes:
>
>  Heiko> Hey,
>
>  Heiko> I was wondering for a while, what are the criterias for patches
>  Heiko> to be applied to buildroot.
>  Heiko> Some of them seem to get applied right away, others were sent in a
>  Heiko> month or two ago and don't. I understand that some probably  
> will never
>  Heiko> get applied, but will there be a "sorry dude, but..." email?
>
> I'm not following any strict procedure, but I do try to sooner or later
> get around to review all patches. Understand that I do this in my spare
> time, so turnaround time can sometimes be slow.

No worries, we all do have the same issue. There are only so many  
hours in a day (must be 48 according to my work load).

> I mainly try to handle the patches in fifo mode, but I do also skim new
> mails. This is both because sometimes updates are sent, and sometimes I
> see trivial patches that can be handled when I have 5 min spare or
> important bugfixes.
>
> It's a big help when other developers help with the review and send
> their ack/nacks (thanks for this!), so those patches are often handled
> earlier than others.
>
> But I can still forget about patches, so if you haven't had any feedback
> for a week or two, please check if it still applies and send a followup
> mail.

I'll try and start nagging. ;-)
I often forget myself about the patches I sent in the past, since I  
keep using them as part of my git repository.

> It might be politically incorrect, but the patch author also does
> matter. If the patch is from a regular contributor who normally does
> good work, then I know I don't need to review in as much details as if
> it's from a complete stranger - So a good way of getting your own
> patches applied faster is to help reviewing other patches ;)

Ha! Now he's trying to make us work... :D

-- 

Regards
   Heiko Zuerker
   http://www.devil-linux.org


----------------------------------------------------------------
This message was sent using IMP, the Internet Messaging Program.

[Buildroot] commiting patches

[Thomas Petazzoni]

Hello,

As this is my first message to the list in 2011: happy new year
everybody. Best wishes for 2011, and let's have some good BR
development during this year :-)

On Thu, 30 Dec 2010 21:54:51 +0100
Peter Korsgaard <jacmet@uclibc.org> wrote:

> I'm not following any strict procedure, but I do try to sooner or
> later get around to review all patches. Understand that I do this in
> my spare time, so turnaround time can sometimes be slow.
> 
> I mainly try to handle the patches in fifo mode, but I do also skim
> new mails. This is both because sometimes updates are sent, and
> sometimes I see trivial patches that can be handled when I have 5 min
> spare or important bugfixes.

I think there is also one criteria that has an impact of the time it
takes for a patch to be applied: what the patch does. If the patch is a
relatively simple fix to a package, a new package, or something that
doesn't have any major impact of Buildroot's infrastructure, or
Buildroot usage, then this patch is likely to be handled/merged sooner
than something that has broader impact.

For example, Heiko, your patch "Improved initramfs and iso target
support" will take quite a bit of time, because it changes quite a few
things, adds new use cases, we have to think whether we want to handle
those use cases, if they are handled the correct way, etc. Such patches
usually take a bit more time to handle than others.

Regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[Buildroot] commiting patches

[Peter Korsgaard]

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:

 Thomas> Hello,
 Thomas> As this is my first message to the list in 2011: happy new year
 Thomas> everybody. Best wishes for 2011, and let's have some good BR
 Thomas> development during this year :-)

Yeah!

 >> I mainly try to handle the patches in fifo mode, but I do also skim
 >> new mails. This is both because sometimes updates are sent, and
 >> sometimes I see trivial patches that can be handled when I have 5
 >> min spare or important bugfixes.

 Thomas> I think there is also one criteria that has an impact of the
 Thomas> time it takes for a patch to be applied: what the patch
 Thomas> does. If the patch is a relatively simple fix to a package, a
 Thomas> new package, or something that doesn't have any major impact of
 Thomas> Buildroot's infrastructure, or Buildroot usage, then this patch
 Thomas> is likely to be handled/merged sooner than something that has
 Thomas> broader impact.

Indeed, that's what I meant when I wrote about trivial patches.

-- 
Bye, Peter Korsgaard

[Buildroot] commiting patches

[Heiko Zuerker]

Quoting Peter Korsgaard <jacmet@uclibc.org>:

>>>>>> "Thomas" == Thomas Petazzoni  
>>>>>> <thomas.petazzoni@free-electrons.com> writes:
>
>  Thomas> Hello,
>  Thomas> As this is my first message to the list in 2011: happy new year
>  Thomas> everybody. Best wishes for 2011, and let's have some good BR
>  Thomas> development during this year :-)
>
> Yeah!

Happy New Year to everyone too.

>  >> I mainly try to handle the patches in fifo mode, but I do also skim
>  >> new mails. This is both because sometimes updates are sent, and
>  >> sometimes I see trivial patches that can be handled when I have 5
>  >> min spare or important bugfixes.
>
>  Thomas> I think there is also one criteria that has an impact of the
>  Thomas> time it takes for a patch to be applied: what the patch
>  Thomas> does. If the patch is a relatively simple fix to a package, a
>  Thomas> new package, or something that doesn't have any major impact of
>  Thomas> Buildroot's infrastructure, or Buildroot usage, then this patch
>  Thomas> is likely to be handled/merged sooner than something that has
>  Thomas> broader impact.
>
> Indeed, that's what I meant when I wrote about trivial patches.

Makes perfect sense.
I also understand that I'm new to BR and still have a lot to figure out.

-- 

Regards
   Heiko Zuerker
   http://www.devil-linux.org


----------------------------------------------------------------
This message was sent using IMP, the Internet Messaging Program.