[Buildroot] [PATCH 0/4] docs: improve submitting patches

[Buildroot] [PATCH 0/4] docs: improve submitting patches

[Arnout Vandecappelle]

I've seen Thomas P. send a few feedback e-mails recently that give basic
explanation to newcomers about how patches should be formatted when
sending them to buildroot. So let's improve the documentation to avoid
this in the future, or to make it easier to just refer to
https://buildroot.org/manual.html#submitting-patches

[Buildroot] [PATCH 1/4] README: add reference to submitting-patches

[Arnout Vandecappelle]

In the hope of improving the quality of patches send by newcomers,
add a reference to the submitting-patches section of the manual
to the top-level README file.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 README | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/README b/README
index e863f0e..c617252 100644
--- a/README
+++ b/README
@@ -21,3 +21,6 @@ Buildroot comes with a basic configuration for a number of boards. Run
 Please feed suggestions, bug reports, insults, and bribes back to the
 buildroot mailing list: buildroot at buildroot.org
 You can also find us on #buildroot on Freenode IRC.
+
+If you would like to contribute patches, please read
+https://buildroot.org/manual.html#submitting-patches
-- 
2.7.0

[Buildroot] [PATCH 2/4] website: add reference to submitting-patches to Contribute tab

[Arnout Vandecappelle]

In the hope of improving the quality of patches send by newcomers,
add a reference to the submitting-patches section of the manual
to the Contribute tab of the website.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
Warning: I haven't been able to test this with the full website js and
css, just with file:///.../docs/website/contribute.html. But I think it
should be OK :-)
---
 docs/website/contribute.html | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/docs/website/contribute.html b/docs/website/contribute.html
index da5700c..721729b 100644
--- a/docs/website/contribute.html
+++ b/docs/website/contribute.html
@@ -22,7 +22,8 @@
 	    patchwork</a>.</li>
 	<li>Working on items from the
 	  <a href="http://www.elinux.org/Buildroot#Todo_list">TODO list</a></li>
-	<li>Submitting your own patches through the
+	<li><a href="http://buildroot.org/manual.html#submitting-patches">Submitting
+	  your own patches</a> through the
 	  <a href="http://lists.buildroot.org/mailman/listinfo/buildroot">mailing list
 	</a></li>
       </ul>
-- 
2.7.0

[Buildroot] [PATCH 3/4] docs/manual/contribute.txt: add section for preparing patch series

[Arnout Vandecappelle]

In subsequent patches, we will add more explanation about how to
prepare patches, so it will be worthwhile to have a separate section
for the series preparation.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
Note: for this patch, I started using
http://dustycloud.org/blog/vcs-friendly-patchable-document-line-wrapping/
If nobody opposes, I propose to require this formatting for the manual
from now on. It will be weird for a while, but in the end I think it
would be A Good Thing (TM). If that formatting had been followed, this
patch, for example, would have a diffstat of +3 instead of the current
+7-4.
---
 docs/manual/contribute.txt | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/docs/manual/contribute.txt b/docs/manual/contribute.txt
index b74897d..e19ba15 100644
--- a/docs/manual/contribute.txt
+++ b/docs/manual/contribute.txt
@@ -182,10 +182,13 @@ _Please, do not attach patches to bugs, send them to the mailing list
 instead_.
 
 If you made some changes to Buildroot and you would like to contribute
-them to the Buildroot project, proceed as follows. Starting from the
-changes committed in your local git view, _rebase_ your development
-branch on top of the upstream tree before generating a patch set. To do
-so, run:
+  them to the Buildroot project, proceed as follows.
+
+==== Preparing a patch series
+
+Starting from the changes committed in your local git view, _rebase_ your
+  development branch on top of the upstream tree before generating a patch set.
+To do so, run:
 
 ---------------------
 $ git fetch --all --tags
-- 
2.7.0

[Buildroot] [PATCH 4/4] docs/manual/contribute.txt: add formatting patches section

[Arnout Vandecappelle]

Thomas P. has sent a few big feedback mails recently that explain how a
patch should be formatted. Indeed, this was not explained much in the
manual, so add a section that explains how patches should be formatted.
This is based heavily on the feedback that Thomas P. gave. Also,
specific examples for new packages and version bumps are added.

This will allow us to refer to
https://buildroot.org/manual.html#submitting-patches
in the future instead of composing long mails.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 docs/manual/contribute.txt | 81 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

diff --git a/docs/manual/contribute.txt b/docs/manual/contribute.txt
index e19ba15..d8a5e6e 100644
--- a/docs/manual/contribute.txt
+++ b/docs/manual/contribute.txt
@@ -184,6 +184,87 @@ instead_.
 If you made some changes to Buildroot and you would like to contribute
   them to the Buildroot project, proceed as follows.
 
+==== The formatting of a patch
+
+We expect patches to be formatted in a specific way.
+This is necessary to make it easy to review patches, to be able to apply
+  them easily to the git repository, to make it easy to find back in the
+  history how and why things have changed, and to make it possible to use
+  +git bisect+ to locate the origin of a problem.
+
+First of all, it is essential that the patch has a good commit message.
+The commit message should start with a separate line with a brief summary
+  of the change, starting with the name of the affected package.
+The body of the commit message should describe _why_ this change is needed,
+  and if necessary also give details about _how_ it was done.
+When writing the commit message, think of how the reviewers will read it,
+  but also think about how you will read it when you look at this change
+  again a few years down the line.
+
+Second, the patch itself should do only one change, but do it completely.
+Two unrelated or weakly related changes should usually be done in two
+  separate patches.
+This usually means that a patch affects only a single package.
+If several changes are related, it is often still possible to split them
+  up in small patches and apply them in a specific order.
+Small patches make it easier to review, and often make it easier to understand
+  afterwards why a change was done.
+However, each patch must be complete.
+It is not allowed that the build is broken when only the first but not the
+  second patch is applied.
+This is necessary to be able to use +git bisect+ afterwards.
+
+Of course, while you're doing your development, you're probably going back
+  and forth between packages, and certainly not committing things
+  immediately in a way that is clean enough for submission.
+So most developers rewrite the history of commits to produce a clean set of
+  commits that is appropriate for submission.
+To do this, you need to use _interactive rebasing_.
+You can learn about it https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History[in
+  the Pro Git book].
+Sometimes, it is even easier to discard you history with +git reset --soft
+  origin/master+ and select individual changes with +git add -i+ or +git add
+  -p+.
+
+Finally, the patch should be signed off.
+This is done by adding +Signed-off-by: Your Real Name <your@email.address>+ at
+  the end of the commit message.
++git commit -s+ does that for you, if configured properly.
+The +Signed-off-by+ tag means that you publish the patch under the Buildroot
+  license (i.e. GPLv2, except for package patches, which have the upstream
+  license), and that you are allowed to do so.
+See http://developercertificate.org/[the Developer Certificate of Origin] for
+  details.
+
+When adding new packages, you should submit every package in a separate patch.
+This patch should have the update to +package/Config.in+, the package
+  +Config.in+ file, the +.mk+ file, the +.hash+ file, any init script, and
+  all package patches.
+If the package has many sub-options, these are sometimes better added as
+  separate follow-up patches.
+The summary line should be something like +<packagename>: new package+.
+The body of the commit message can be empty for simple packages, or it can
+  contain the description of the package (like the Config.in help text).
+If anything special has to be done to build the package, this should also
+  be explained explicitly in the commit message body.
+
+When you bump a package to a new version, you should also submit a separate
+  patch for each package.
+Don't forget to update the +.hash+ file, or add it if it doesn't exist yet.
+Also don't forget to check if the +_LICENSE+ and +_LICENSE_FILES+ are still
+  valid.
+The summary line should be something like +<packagename>: bump to version <new
+  version>+.
+If the new version only contains security updates compared to the existing one,
+  the summary should be +<packagename>: security bump to version <new version>+
+  and the commit message body should show the CVE numbers that are fixed.
+If some package patches can be removed in the new version, it should be
+  explained explicitly why they can be removed, preferably with the upstream
+  commit ID.
+Also any other required changes should be explained explicitly, like configure
+  options that no longer exist or are no longer needed.
+
+
 ==== Preparing a patch series
 
 Starting from the changes committed in your local git view, _rebase_ your
-- 
2.7.0

[Buildroot] [PATCH 1/4] README: add reference to submitting-patches

[Thomas Petazzoni]

Dear Arnout Vandecappelle (Essensium/Mind),

On Mon, 1 Feb 2016 18:45:14 +0100, Arnout Vandecappelle
(Essensium/Mind) wrote:
> In the hope of improving the quality of patches send by newcomers,
> add a reference to the submitting-patches section of the manual
> to the top-level README file.
> 
> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> ---
>  README | 3 +++
>  1 file changed, 3 insertions(+)

Applied, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 2/4] website: add reference to submitting-patches to Contribute tab

[Thomas Petazzoni]

Dear Arnout Vandecappelle (Essensium/Mind),

On Mon, 1 Feb 2016 18:45:15 +0100, Arnout Vandecappelle
(Essensium/Mind) wrote:
> In the hope of improving the quality of patches send by newcomers,
> add a reference to the submitting-patches section of the manual
> to the Contribute tab of the website.
> 
> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> ---
> Warning: I haven't been able to test this with the full website js and
> css, just with file:///.../docs/website/contribute.html. But I think it
> should be OK :-)
> ---
>  docs/website/contribute.html | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Applied, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 4/4] docs/manual/contribute.txt: add formatting patches section

[Yegor Yefremov]

On Mon, Feb 1, 2016 at 6:45 PM, Arnout Vandecappelle (Essensium/Mind)
<arnout@mind.be> wrote:
> Thomas P. has sent a few big feedback mails recently that explain how a
> patch should be formatted. Indeed, this was not explained much in the
> manual, so add a section that explains how patches should be formatted.
> This is based heavily on the feedback that Thomas P. gave. Also,
> specific examples for new packages and version bumps are added.
>
> This will allow us to refer to
> https://buildroot.org/manual.html#submitting-patches
> in the future instead of composing long mails.
>
> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> ---
>  docs/manual/contribute.txt | 81 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 81 insertions(+)
>
> diff --git a/docs/manual/contribute.txt b/docs/manual/contribute.txt
> index e19ba15..d8a5e6e 100644
> --- a/docs/manual/contribute.txt
> +++ b/docs/manual/contribute.txt
> @@ -184,6 +184,87 @@ instead_.
>  If you made some changes to Buildroot and you would like to contribute
>    them to the Buildroot project, proceed as follows.
>
> +==== The formatting of a patch
> +
> +We expect patches to be formatted in a specific way.
> +This is necessary to make it easy to review patches, to be able to apply
> +  them easily to the git repository, to make it easy to find back in the
> +  history how and why things have changed, and to make it possible to use
> +  +git bisect+ to locate the origin of a problem.
> +
> +First of all, it is essential that the patch has a good commit message.
> +The commit message should start with a separate line with a brief summary
> +  of the change, starting with the name of the affected package.
> +The body of the commit message should describe _why_ this change is needed,
> +  and if necessary also give details about _how_ it was done.
> +When writing the commit message, think of how the reviewers will read it,
> +  but also think about how you will read it when you look at this change
> +  again a few years down the line.
> +
> +Second, the patch itself should do only one change, but do it completely.
> +Two unrelated or weakly related changes should usually be done in two
> +  separate patches.
> +This usually means that a patch affects only a single package.
> +If several changes are related, it is often still possible to split them
> +  up in small patches and apply them in a specific order.
> +Small patches make it easier to review, and often make it easier to understand
> +  afterwards why a change was done.
> +However, each patch must be complete.
> +It is not allowed that the build is broken when only the first but not the
> +  second patch is applied.
> +This is necessary to be able to use +git bisect+ afterwards.
> +
> +Of course, while you're doing your development, you're probably going back
> +  and forth between packages, and certainly not committing things
> +  immediately in a way that is clean enough for submission.
> +So most developers rewrite the history of commits to produce a clean set of
> +  commits that is appropriate for submission.
> +To do this, you need to use _interactive rebasing_.
> +You can learn about it https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History[in
> +  the Pro Git book].
> +Sometimes, it is even easier to discard you history with +git reset --soft
> +  origin/master+ and select individual changes with +git add -i+ or +git add
> +  -p+.
> +
> +Finally, the patch should be signed off.
> +This is done by adding +Signed-off-by: Your Real Name <your@email.address>+ at
> +  the end of the commit message.
> ++git commit -s+ does that for you, if configured properly.
> +The +Signed-off-by+ tag means that you publish the patch under the Buildroot
> +  license (i.e. GPLv2, except for package patches, which have the upstream
> +  license), and that you are allowed to do so.
> +See http://developercertificate.org/[the Developer Certificate of Origin] for
> +  details.
> +
> +When adding new packages, you should submit every package in a separate patch.
> +This patch should have the update to +package/Config.in+, the package
> +  +Config.in+ file, the +.mk+ file, the +.hash+ file, any init script, and
> +  all package patches.
> +If the package has many sub-options, these are sometimes better added as
> +  separate follow-up patches.
> +The summary line should be something like +<packagename>: new package+.
> +The body of the commit message can be empty for simple packages, or it can
> +  contain the description of the package (like the Config.in help text).
> +If anything special has to be done to build the package, this should also
> +  be explained explicitly in the commit message body.
> +
> +When you bump a package to a new version, you should also submit a separate
> +  patch for each package.
> +Don't forget to update the +.hash+ file, or add it if it doesn't exist yet.
> +Also don't forget to check if the +_LICENSE+ and +_LICENSE_FILES+ are still
> +  valid.
> +The summary line should be something like +<packagename>: bump to version <new
> +  version>+.

For now bump message looks so:

packagename: bump to <new version>

should we keep it this way, i.e. without the word "version"?

> +If the new version only contains security updates compared to the existing one,
> +  the summary should be +<packagename>: security bump to version <new version>+

same here

> +  and the commit message body should show the CVE numbers that are fixed.
> +If some package patches can be removed in the new version, it should be
> +  explained explicitly why they can be removed, preferably with the upstream
> +  commit ID.
> +Also any other required changes should be explained explicitly, like configure
> +  options that no longer exist or are no longer needed.
> +
> +
>  ==== Preparing a patch series
>
>  Starting from the changes committed in your local git view, _rebase_ your
> --
> 2.7.0
>
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

[Buildroot] [PATCH 4/4] docs/manual/contribute.txt: add formatting patches section

[Arnout Vandecappelle]

On 08-02-16 11:48, Yegor Yefremov wrote:
> On Mon, Feb 1, 2016 at 6:45 PM, Arnout Vandecappelle (Essensium/Mind)
> <arnout@mind.be> wrote:

[snip]
>> +The summary line should be something like +<packagename>: bump to version <new
>> +  version>+.
> 
> For now bump message looks so:
> 
> packagename: bump to <new version>
> 
> should we keep it this way, i.e. without the word "version"?

 Since 2015.02, we have:

 89 times '<pkg>: bump version'
382 times '<pkg>: bump version to ...'
816 times '<pkg>: bump to version ...'


 Regards,
 Arnout

> 
>> +If the new version only contains security updates compared to the existing one,
>> +  the summary should be +<packagename>: security bump to version <new version>+
> 
> same here
> 
>> +  and the commit message body should show the CVE numbers that are fixed.
>> +If some package patches can be removed in the new version, it should be
>> +  explained explicitly why they can be removed, preferably with the upstream
>> +  commit ID.
>> +Also any other required changes should be explained explicitly, like configure
>> +  options that no longer exist or are no longer needed.
>> +
>> +
>>  ==== Preparing a patch series
>>
>>  Starting from the changes committed in your local git view, _rebase_ your
>> --
>> 2.7.0
>>
>> _______________________________________________
>> buildroot mailing list
>> buildroot at busybox.net
>> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
Arnout Vandecappelle      arnout dot vandecappelle at essensium dot com
Senior Embedded Software Architect . . . . . . +32-478-010353 (mobile)
Essensium, Mind division . . . . . . . . . . . . . . http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium . . . . . BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH 3/4] docs/manual/contribute.txt: add section for preparing patch series

[Yegor Yefremov]

On Mon, Feb 1, 2016 at 6:45 PM, Arnout Vandecappelle (Essensium/Mind)
<arnout@mind.be> wrote:
> In subsequent patches, we will add more explanation about how to
> prepare patches, so it will be worthwhile to have a separate section
> for the series preparation.
>
> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

Reviewed-by: Yegor Yefremov <yegorslists@googlemail.com>

> ---
> Note: for this patch, I started using
> http://dustycloud.org/blog/vcs-friendly-patchable-document-line-wrapping/
> If nobody opposes, I propose to require this formatting for the manual
> from now on. It will be weird for a while, but in the end I think it
> would be A Good Thing (TM). If that formatting had been followed, this
> patch, for example, would have a diffstat of +3 instead of the current
> +7-4.
> ---
>  docs/manual/contribute.txt | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/docs/manual/contribute.txt b/docs/manual/contribute.txt
> index b74897d..e19ba15 100644
> --- a/docs/manual/contribute.txt
> +++ b/docs/manual/contribute.txt
> @@ -182,10 +182,13 @@ _Please, do not attach patches to bugs, send them to the mailing list
>  instead_.
>
>  If you made some changes to Buildroot and you would like to contribute
> -them to the Buildroot project, proceed as follows. Starting from the
> -changes committed in your local git view, _rebase_ your development
> -branch on top of the upstream tree before generating a patch set. To do
> -so, run:
> +  them to the Buildroot project, proceed as follows.
> +
> +==== Preparing a patch series
> +
> +Starting from the changes committed in your local git view, _rebase_ your
> +  development branch on top of the upstream tree before generating a patch set.
> +To do so, run:
>
>  ---------------------
>  $ git fetch --all --tags
> --
> 2.7.0
>
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

[Buildroot] [PATCH 4/4] docs/manual/contribute.txt: add formatting patches section

[Yegor Yefremov]

On Mon, Feb 8, 2016 at 3:33 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
>
>
> On 08-02-16 11:48, Yegor Yefremov wrote:
>> On Mon, Feb 1, 2016 at 6:45 PM, Arnout Vandecappelle (Essensium/Mind)
>> <arnout@mind.be> wrote:
>
> [snip]
>>> +The summary line should be something like +<packagename>: bump to version <new
>>> +  version>+.
>>
>> For now bump message looks so:
>>
>> packagename: bump to <new version>
>>
>> should we keep it this way, i.e. without the word "version"?
>
>  Since 2015.02, we have:
>
>  89 times '<pkg>: bump version'
> 382 times '<pkg>: bump version to ...'
> 816 times '<pkg>: bump to version ...'

OK. Then I'll change to the new scheme :-)

Reviewed-by: Yegor Yefremov <yegorslists@googlemail.com>

[Buildroot] [PATCH 3/4] docs/manual/contribute.txt: add section for preparing patch series

[Thomas Petazzoni]

Dear Arnout Vandecappelle (Essensium/Mind),

On Mon, 1 Feb 2016 18:45:16 +0100, Arnout Vandecappelle
(Essensium/Mind) wrote:
> In subsequent patches, we will add more explanation about how to
> prepare patches, so it will be worthwhile to have a separate section
> for the series preparation.
> 
> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> ---
> Note: for this patch, I started using
> http://dustycloud.org/blog/vcs-friendly-patchable-document-line-wrapping/
> If nobody opposes, I propose to require this formatting for the manual
> from now on. It will be weird for a while, but in the end I think it
> would be A Good Thing (TM). If that formatting had been followed, this
> patch, for example, would have a diffstat of +3 instead of the current
> +7-4.
> ---
>  docs/manual/contribute.txt | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)

Applied to master, after rewrapping to our normal standards, as
discussed on IRC. I don't really like the proposed new formatting (but
I can get convinced), but mainly I didn't like the fact that it was
only applied to one part of the manual.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 4/4] docs/manual/contribute.txt: add formatting patches section

[Thomas Petazzoni]

Dear Arnout Vandecappelle (Essensium/Mind),

On Mon, 1 Feb 2016 18:45:17 +0100, Arnout Vandecappelle
(Essensium/Mind) wrote:
> Thomas P. has sent a few big feedback mails recently that explain how a
> patch should be formatted. Indeed, this was not explained much in the
> manual, so add a section that explains how patches should be formatted.
> This is based heavily on the feedback that Thomas P. gave. Also,
> specific examples for new packages and version bumps are added.
> 
> This will allow us to refer to
> https://buildroot.org/manual.html#submitting-patches
> in the future instead of composing long mails.
> 
> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> ---
>  docs/manual/contribute.txt | 81 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 81 insertions(+)

Applied to master, thanks. Same as PATCH 3/4 in terms of formatting,
I've changed it back to our normal practice.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com