[Buildroot] [PATCH] docs/manual: standardize a bit more the formatting of commit titles

[Buildroot] [PATCH] docs/manual: standardize a bit more the formatting of commit titles

[Thomas Petazzoni]

Currently, our commit titles are not very well standardized, and it
would be great to standardize them a little bit more. A number of
people use "<pkg>: " as prefix, others use "package/<pkg>: ". Some
people start the rest of the commit title (after the prefix) with an
upper-case letter, some with a lower-case letter.

In an attempt to standardize this, this commit updates the manual with
some examples of good commit titles.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 docs/manual/contribute.txt | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/docs/manual/contribute.txt b/docs/manual/contribute.txt
index 60bfb961f0..5530ce1546 100644
--- a/docs/manual/contribute.txt
+++ b/docs/manual/contribute.txt
@@ -194,14 +194,29 @@ 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
+brief summary of the change, prefixed by the area touched by the
+patch. A few examples of good commit titles:
+
+* +package/linuxptp: bump version to 2.0+
+
+* +configs/imx23evk: bump Linux version to 4.19+
+
+* +package/pkg-generic: postpone evaluation of dependency conditions+
+
+* +boot/uboot: needs host-{flex,bison}+
+
+* +support/testing: add python-ubjson tests+
+
+The description that follows the prefix should start with a lower case
+letter (i.e "bump", "needs", "postpone", "add" in the above examples).
+
+Second, 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
+Third, 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
-- 
2.19.1

[Buildroot] [PATCH] docs/manual: standardize a bit more the formatting of commit titles

[Carlos Santos]

> From: "Thomas Petazzoni" <thomas.petazzoni@bootlin.com>
> To: "buildroot" <buildroot@buildroot.org>
> Cc: "Thomas De Schampheleire" <thomas.de_schampheleire@nokia.com>, "Thomas Petazzoni" <thomas.petazzoni@bootlin.com>
> Sent: S?bado, 24 de novembro de 2018 8:19:03
> Subject: [Buildroot] [PATCH] docs/manual: standardize a bit more the formatting of commit titles

> Currently, our commit titles are not very well standardized, and it
> would be great to standardize them a little bit more. A number of
> people use "<pkg>: " as prefix, others use "package/<pkg>: ". Some
> people start the rest of the commit title (after the prefix) with an
> upper-case letter, some with a lower-case letter.
> 
> In an attempt to standardize this, this commit updates the manual with
> some examples of good commit titles.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
> docs/manual/contribute.txt | 21 ++++++++++++++++++---
> 1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/docs/manual/contribute.txt b/docs/manual/contribute.txt
> index 60bfb961f0..5530ce1546 100644
> --- a/docs/manual/contribute.txt
> +++ b/docs/manual/contribute.txt
> @@ -194,14 +194,29 @@ 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
> +brief summary of the change, prefixed by the area touched by the
> +patch. A few examples of good commit titles:
> +
> +* +package/linuxptp: bump version to 2.0+

Looks good, but isn't "package/" a bit superfluous, since Most of the
commits are for packages? It also goes against the tradition:

$ git log --oneline origin/master | grep -i '^.......... [^ /]*: bump' |wc -l
7644
$ git log --oneline origin/master | grep -i '^.......... [^ ]*/[^ ]*: bump' |wc -l
2296

BTW, I want the bike shed red.

> +
> +* +configs/imx23evk: bump Linux version to 4.19+
> +
> +* +package/pkg-generic: postpone evaluation of dependency conditions+
> +
> +* +boot/uboot: needs host-{flex,bison}+
> +
> +* +support/testing: add python-ubjson tests+
> +
> +The description that follows the prefix should start with a lower case
> +letter (i.e "bump", "needs", "postpone", "add" in the above examples).
> +
> +Second, 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
> +Third, 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
> --
> 2.19.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

Reviewed-by: Carlos Santos <casantos@datacom.com.br>

-- 
Carlos Santos (Casantos) - DATACOM, P&D
?Marched towards the enemy, spear upright, armed with the certainty
that only the ignorant can have.? ? Epitaph of a volunteer

[Buildroot] [PATCH] docs/manual: standardize a bit more the formatting of commit titles

[Peter Korsgaard]

>>>>> "Carlos" == Carlos Santos <casantos@datacom.com.br> writes:

Hi,

 >> 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
 >> +brief summary of the change, prefixed by the area touched by the
 >> +patch. A few examples of good commit titles:
 >> +
 >> +* +package/linuxptp: bump version to 2.0+

 > Looks good, but isn't "package/" a bit superfluous, since Most of the
 > commits are for packages? It also goes against the tradition:

 > $ git log --oneline origin/master | grep -i '^.......... [^ /]*: bump' |wc -l
 > 7644
 > $ git log --oneline origin/master | grep -i '^.......... [^ ]*/[^ ]*: bump' |wc -l
 > 2296

I also personally prefer without package/, but consistency and
documentation in the manual is nice, so Ok for me.

Acked-by: Peter Korsgaard <peter@korsgaard.com>


 > BTW, I want the bike shed red.

Blue!

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH] docs/manual: standardize a bit more the formatting of commit titles

[Peter Korsgaard]

>>>>> "Peter" == Peter Korsgaard <peter@korsgaard.com> writes:

>>>>> "Carlos" == Carlos Santos <casantos@datacom.com.br> writes:
 > Hi,

 >>> 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
 >>> +brief summary of the change, prefixed by the area touched by the
 >>> +patch. A few examples of good commit titles:
 >>> +
 >>> +* +package/linuxptp: bump version to 2.0+

 >> Looks good, but isn't "package/" a bit superfluous, since Most of the
 >> commits are for packages? It also goes against the tradition:

 >> $ git log --oneline origin/master | grep -i '^.......... [^ /]*: bump' |wc -l
 >> 7644
 >> $ git log --oneline origin/master | grep -i '^.......... [^ ]*/[^ ]*: bump' |wc -l
 >> 2296

 > I also personally prefer without package/, but consistency and
 > documentation in the manual is nice, so Ok for me.

Next to the purely subjective preference, there is also the issue of
running into the max subject length with long package paths, E.G.:

package/freescale-imx/kernel-module-imx-gpu-viv: <blah>
package/x11r7/xfont_font-bh-lucidatypewriter-100dpi: <blih>
package/python-backports-shutil-get-terminal-size: <bloh>

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH] docs/manual: standardize a bit more the formatting of commit titles

[Thomas Petazzoni]

Hello,

On Sat, 24 Nov 2018 23:16:33 -0200 (BRST), Carlos Santos wrote:

> Looks good, but isn't "package/" a bit superfluous, since Most of the
> commits are for packages? It also goes against the tradition:
> 
> $ git log --oneline origin/master | grep -i '^.......... [^ /]*: bump' |wc -l
> 7644
> $ git log --oneline origin/master | grep -i '^.......... [^ ]*/[^ ]*: bump' |wc -l
> 2296

Yes, I agree the tradition has been without package/. I myself used to
prefer without package/. However, it is not very consistent, as then
when we have boot/uboot, is it about package/boot/uboot ? Or when we
have core: <something>, is it about the package "core" ?

While I was personally not adding package/, I saw others doing it, and
ultimately found it more consistent.

> BTW, I want the bike shed red.

Red is not too bad :-)

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH] docs/manual: standardize a bit more the formatting of commit titles

[Yann E. MORIN]

Thomas, All,

On 2018-11-24 11:19 +0100, Thomas Petazzoni spake thusly:
> Currently, our commit titles are not very well standardized, and it
> would be great to standardize them a little bit more. A number of
> people use "<pkg>: " as prefix, others use "package/<pkg>: ". Some
> people start the rest of the commit title (after the prefix) with an
> upper-case letter, some with a lower-case letter.
> 
> In an attempt to standardize this, this commit updates the manual with
> some examples of good commit titles.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
>  docs/manual/contribute.txt | 21 ++++++++++++++++++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/docs/manual/contribute.txt b/docs/manual/contribute.txt
> index 60bfb961f0..5530ce1546 100644
> --- a/docs/manual/contribute.txt
> +++ b/docs/manual/contribute.txt
> @@ -194,14 +194,29 @@ 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
> +brief summary of the change, prefixed by the area touched by the
> +patch. A few examples of good commit titles:
> +
> +* +package/linuxptp: bump version to 2.0+

Aha, so you finally saw the light! ;-)

When I add 'package/' as a prefix, it is not because it is in the
package subdirectory, but because it is touching a package, as opposed
to something else, like core stuff or package infrastructures.

The prefix is a 'semantic' prefix, not a path prefix. A path prefix
serves no purpose in and of itself.

> +* +configs/imx23evk: bump Linux version to 4.19+
> +
> +* +package/pkg-generic: postpone evaluation of dependency conditions+

There, I would have sued 'infra/' as a prefix, because it is touching
the pkg0generic infrastructure, not a package.

> +* +boot/uboot: needs host-{flex,bison}+

And there, I would have used 'package'/'.

> +* +support/testing: add python-ubjson tests+

Yes.

> +The description that follows the prefix should start with a lower case
> +letter (i.e "bump", "needs", "postpone", "add" in the above examples).

    The description that follows the prefix should be all in lower case,
    and start with a verb, if possible in the imperative form ("add",
    "postpone", "bump"), or in the third person of the present ("needs",
    "should be")

Otherwise, OK.

Oh, by the way, the shed ought to be blue. ;-)

Regards,
Yann E. MORIN.

> +Second, 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
> +Third, 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
> -- 
> 2.19.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  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] [PATCH] docs/manual: standardize a bit more the formatting of commit titles

[Thomas Petazzoni]

Hello,

On Sun, 25 Nov 2018 17:17:22 +0100, Yann E. MORIN wrote:

> When I add 'package/' as a prefix, it is not because it is in the
> package subdirectory, but because it is touching a package, as opposed
> to something else, like core stuff or package infrastructures.
> 
> The prefix is a 'semantic' prefix, not a path prefix. A path prefix
> serves no purpose in and of itself.

I don't quite agree. If we really want to use 'semantic' prefix, then
we should do:

	package: foo: bump version to ...

Using 'package/foo' really looks way too much like a path, and
therefore...

> There, I would have sued 'infra/' as a prefix, because it is touching
> the pkg0generic infrastructure, not a package.

this doesn't make much sense to me, because there is nothing named
infra/pkg-generic in the tree. "infra: pkg-generic: " would be OK, but
we have never really used such 'semantic' naming so far.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH] docs/manual: standardize a bit more the formatting of commit titles

[Yann E. MORIN]

Thomas, All,

On 2018-11-25 21:09 +0100, Thomas Petazzoni spake thusly:
> On Sun, 25 Nov 2018 17:17:22 +0100, Yann E. MORIN wrote:
> > When I add 'package/' as a prefix, it is not because it is in the
> > package subdirectory, but because it is touching a package, as opposed
> > to something else, like core stuff or package infrastructures.
> > 
> > The prefix is a 'semantic' prefix, not a path prefix. A path prefix
> > serves no purpose in and of itself.
> 
> I don't quite agree. If we really want to use 'semantic' prefix, then
> we should do:
> 
> 	package: foo: bump version to ...
> 
> Using 'package/foo' really looks way too much like a path, and
> therefore...
> 
> > There, I would have sued 'infra/' as a prefix, because it is touching
> > the pkg0generic infrastructure, not a package.
> 
> this doesn't make much sense to me, because there is nothing named
> infra/pkg-generic in the tree. "infra: pkg-generic: " would be OK,

I'd be OK to use your proposal using the 'prefix: name: what" (even
though I find it ugly, but that's probably because I haven't been
totally tainted by the kernel yet ;-) )

So, for example:

    package: busybox: bump version to 1.29.3
    package: xdriver_xf86-video-siliconmotion: bump version to 1.7.9
    package: uboot: switch to kconfig by default
    infra: pkg-generic: fix show-build-order stdout pollution
    core: legal: add package dependencies with licenses to the manifest
    fs: common: drop intermediate tarball
    fs: ext2: add option for lz4 compression
    testing: add test for the f2fs filesystem support
    [...]
    arch: arm: add FPv5 support for cortex-m7
    docs: manual: add external.desc to list of files needed for BR2_EXTERNAL
    docs: website: update for 2018.11-rc1
    misc: anything else goes here

> but
> we have never really used such 'semantic' naming so far.

Actually, that is exactly what I've been doing all those years.

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] [PATCH] docs/manual: standardize a bit more the formatting of commit titles

[Thomas Petazzoni]

Hello,

On Mon, 26 Nov 2018 21:20:42 +0100, Yann E. MORIN wrote:

> I'd be OK to use your proposal using the 'prefix: name: what" (even
> though I find it ugly, but that's probably because I haven't been
> totally tainted by the kernel yet ;-) )

Actually, I wasn't proposing to use "foo: bar: what", but to keep
"path/to: what" :-)

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH] docs/manual: standardize a bit more the formatting of commit titles

[Yann E. MORIN]

Thomas, All,

On 2018-11-26 21:25 +0100, Thomas Petazzoni spake thusly:
> On Mon, 26 Nov 2018 21:20:42 +0100, Yann E. MORIN wrote:
> > I'd be OK to use your proposal using the 'prefix: name: what" (even
> > though I find it ugly, but that's probably because I haven't been
> > totally tainted by the kernel yet ;-) )
> 
> Actually, I wasn't proposing to use "foo: bar: what", but to keep
> "path/to: what" :-)

And I am arguing that usig prefixes is better than paths. ;-)

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] [PATCH] docs/manual: standardize a bit more the formatting of commit titles

[Peter Korsgaard]

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@bootlin.com> writes:

 > Currently, our commit titles are not very well standardized, and it
 > would be great to standardize them a little bit more. A number of
 > people use "<pkg>: " as prefix, others use "package/<pkg>: ". Some
 > people start the rest of the commit title (after the prefix) with an
 > upper-case letter, some with a lower-case letter.

 > In an attempt to standardize this, this commit updates the manual with
 > some examples of good commit titles.

 > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

We had some discussion if this is the best format to use, but it is
largely a matter of personal preferences, and having a documented
standard is better than none.

So committed, thanks.

-- 
Bye, Peter Korsgaard