[linux-dvb] [RFC] Reviewed development procedures

Trent Piepho xyzzy at speakeasy.org
Tue Mar 6 22:29:44 CET 2007


On Tue, 6 Mar 2007, Michael Krufky wrote:
> #2, I have made some corrections to the document.  Please see these corrections
> inline below.

I'll add some too.

> > +drivers. They are a volunteer, non-remunerated, group of people that
>
>                                   ^^^^^^^^^^^^^^^ ???

[I believe that unremunerated is the correct word, "Contributing one's time
without pay."]

                         ^volunteer and unremunerated group of people


> > +2. Git trees related with v4l/dvb development
           ^trees' relationhsips with
> > +   ==========================================
> > +
> > +The main kernel trees are hosted at http://git.kernel.org. Each tree
> > +is owned by a maintainer.
> > +
> > +The main kernel tree is owned by Linus Torvalds, being located at:
						      ^ and is located at:

> > +	http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git
> > +
> > +The subsystem master tree, is owned by the subsystem maintainer (Mauro
>
>                                              ^ v4l
                              ^ remove comma
>
> > +Carvalho Chehab), being located at:
                      ^and is located at:
> > +	http://git.kernel.org/?p=linux/kernel/git/mchehab/v4l-dvb.git
> > +
> > +Mike Krufky maintains a number of backport trees, having patches
>
> ^^^ Michael Krufky
                                                      ^which have patches

> > +3. Mercurial trees used on v4l/dvb development
                             ^^ for

> > +V4L/DVB driver development is hosted at http://linuxtv.org. There are a
> > +number of trees there, owned by each developer of the LinuxTV team. Each
                          ^^ each owned by a developer on the

> > +developer use to keep and maintain one or more trees there with their
> > +development stuff.
^^ [I'd delete this sentence.]

> > +When a developer believes that he have patches done to be merged, he
>
>                                     ^ has
                                           ^finished patches to be

> > +send a request to developers ML and to the subsystem maintainer. The
>
>   ^sends
                      ^the developers' mailing list
>
> > +patches are analised and, if they look ok, merged at the master
>
>               ^ analyzed
                                                      ^into ??

> > +It is a good practice that each developer will have at least one tree
       ^is good practice

> > +called 'v4l-dvb', keeping their stable patches, periodically updating
> > +his tree with master's patches.
                    ^, which keeps their patches, and periodically update
this tree with the master tree's patches.

> > +4. Community best practices
> > +   ========================
> > +
> > +From the accumulated experience, there are some basic rules that should
          ^^^ remove the

> > +b) All commits at mercurial trees should have a consistent message,
> > +   describing the patch. This is done by using:
> > +
> > +	make commit
> > +
> > +   This will run some scripts that will check changed files, seeking for
                                       ^remove will              ^correct bad whitespace
> > +   bad whitespace practices and will prepare the last Signed-off-by
                                     ^remove will

> > +   field, as described below. A good comment will have a brief
> > +   description at the first line, up to 68 characters wide, the patch author's
                                                                 ^a blank line, the patch author's

> > +   name at the second line, and a brief description, with no more than
            ^on the third line, a blank line, and then a brief description

> > +   80 chars by line, like:
                ^per     ^e.g.

[Should we have the make commit script check the line length limits?]

> > +   All lines starting with # will be removed by make commit stripts.
And all lines starting with HG: will be removed by Mercurial.

> > +
> > +   *WARNING* Be careful not to leave the first line blank, otherwise hg
> > +   will leave subject in blank.
                   ^the subject blank.

> > +   From: line shouldn't be suppressed, since it will be used when
                               ^ omitted                    ^for the patch author
        when converting to -git.

> > +c) For "make commit" to work properly, the HGUSER shell environment var
> > +   should be defined (replacing the names at the left):
> > +
> > +	HGUSER="My Name <myemail at mysite.com>"

If HGUSER is not set, then, if present, the username defined in the user's
~/.hgrc file will be used.  Use of the .hgrc file is preferred over the
HGUSER variable according to the hg man page.

This name will be used for both the user metadata in the Hg commit log and
the initial values of the "From:" and "Signed-off-by:" lines in the
pre-made commit message.

> > +   It is also possible to use a different login name at the site
                                                         ^for
> > +   hosting the repo, by using:
> > +
> > +	CHANGE_LOG_LOGIN=my_log_name
> > +
> > +   With this, "make push" will use my_log_name, instead of the name of
> > +   the current unix user.
> > +
> > +   Don't forget to export the vars at shell, with something like:
> > +
> > +	export CHANGE_LOG_LOGIN HGUSER
> > +
> > +   It is strongly recommended to have those lines at .bashrc or .profile.
                                                      ^in

> > +
> > +d) All patches shall have a Developers Certificate of Origin
> > +   version 1.1 at commit log or email description, signed by the
                   ^in the commit

> > +   patch authors, as postulated at kernel's source at:
                                    ^in the Linux Kernel source at:

> > +
> > +	Documentation/SubmittingPatches
> > +
> > +   This is done by using Signed-off-by: fields at hg commit message.
                       ^adding Signed-off-by: fields to the hg commit message.

> > +
> > +   It is not acceptable to use fake signatures like:
                                                  ^, e.g.:

> > +	Signed-off-by: Fake me <me at snakeoilcompany.com>
> > +
> > +   The email should be a valid one.
                 ^must

> > +   Each one that is at kernel patch submission chain (driver author
           ^person who is in the kernel patch sumbission chain

> > +   and/or driver maintainer, subsystem/core maintainers, etc) will
> > +   review each patch. If they agree, the patch will be added on their
                                                                 ^to
> > +   trees and signed by they. Otherwise, they may comment the patch,
>            ^^ and signed.
                                                            ^on the patch,
> > +   asking for some review.

> > +e) Although not documented at kernel's Documentation/, a common kernel
> > +   practice is to use Acked-by: meta tag.
                         ^ an Acked-by: tag.

> > +   An Acked-by usually means that the acked person didn't wrote the patch,
>                                                             ^write
                  ^ tag usually

> > +   nor is at the chain responsible to send the patch to kernel, but
              ^in                      ^for sending

> > +   tested or reviewed it and agrees that the patch is good.
                          ^the patch and agreed that it was good.

> > +   A patch acked-by can be added at hg trees, if received by each tree
> > +   maintainer. It is also common to receive acks after having a patch

[Is there anyway to add an ack without deleting and re-creating the tree?]

> > +   inserted at master repository. In this case, the ack will be added
> > +   only at -git tree.

[How does one record that the v4l subsytem maintainer should add these acks?]

> > +f)  If the patch also affects other parts of kernel (like alsa
> > +    or i2c), it is required that, at upstream submitting, the patch
                                      ^when submitting upstream,

> > +    When submitting a patch via e-mail, the better is to direclty the
> > +    interested people add at email's CC field.
>
>      ^ When submitting a patch via email, it is better to copy all interested
>        parties [directly], by adding them as [CCs] to the email itself.

> > +    other subsystem maintainers. So, the best practice is to first ask
> > +    their acks, then commit at the development tree or send it via email
       ^for their acks,         ^to                             ^the patch

> > +    already with a proper Acked-by:
        ^with their Acked-by: already included.

> > +g) Sometimes, mainstream changes do affect v4l-dvb tree, and requires
                                      ^affect the                ^must be

> > +   to backport some kernel patches at the tree. This kind of commit
        ^backported the v4l-dvb tree.

> > +   at mercurial tree should follow the rules above and should also have
       ^to the

> > +   a line like:
       ^the line:
> > +
> > +	kernel-sync:
> > +
> > +   Patches with such lines will not be submitted upstream.
                         ^a line

> > +h) Sometimes it is necessary to introduce some testing code inside a
> > +   module or remove parts that are not yet finished. Also, compatibility
> > +   tests may be required to provide backporting.
> > +
> > +   To allow compatibility tests, "compat.h" should be included at the
> > +   files under v4l-dvb tree. This header also includes linux/version.h.

[This is not true anymore.  linux/version.h is included via the build system
cflags, it is not necessary to include "compat.h" to use a kernel version test.
Is it best if compat.h is included after all <*.h> includes and before any "*.h"
includes.]

> > +   To include testing code, #if 0 or #if 1 may be used. If this code
> > +   is meant to go also to kernel, this struct should be used:
                                    ^ a comment with the word "keep" should be used, e.g:
> > +
> > +    #if 0 /* keep */
> > +	or
> > +    #if 1 /* keep */
> > +
> > +   The kernel submit scripts will remove the compatibility codes, the
> > +   tests for specific kernel versions and the #if 0/1 that doesn't have
> > +   the "keep meta tag".

	See the file v4l/scripts/gentree.pl for a more complete description
	of what kind of code will be kept and what kind will be removed.

> > +i) To import contributed stuff on a developers tree, on mercurial a
                                        ^developer's, a
>                                  ^to

> > +   script is provided at the tree. This allows an easy import of a
                 ^provided.                                          ^remove a

> > +    that can be applied/unapplied for testing. mailimport trusts on it to work,
> > +    so, this extension should be enabled for mailimport script to work.

         In order to work, mailimport requires that this extension is enabled.

> > +5. Knowing about newer patches commited at master hg repository
> > +   ============================================================
> > +
> > +There's a patchbomb script at linuxtv.org that will send one copy of
> > +each patch applied to -hg tree to:
> > +
> > +1) the subscribers of video4linux-cvs at linuxtv.org mailing list (*);
       ^The              ^the

> > +
> > +2) the patch author (as stated at From: field at the middle of the
       ^The                        ^on the        ^in the patch comments

> > +   comments);
> > +
> > +3) to the patch committer (the "user" at hg metadata);
       ^The patch committer

> > +
> > +4) to all people with Signed-off-by:, Acked-by:, or CC: metadata clause
       ^All people
> > +   at the patch's comment.
       ^in

> > +If, for some reason, there's no "From:" metatag (or it is at the first
                                                              ^on
> > +line, instead of the second one), sometimes the script may fail, maybe
> > +filling patch authorship wrongly. So people should take care of properly
                                                                 ^to
> > +committing patches with "From:".
    ^commit

> > +(*) yes, we know that this is really a very bad name for this
         ^Yes
> > +    list, since:
> > +        a) We don't use CVS anymore;
> > +        b) The script will post at this list any HG patches applied to
> > +           the main hg trees, including dvb-apps and v4l-dvb patches.
> > +
> > +6. Patch handling for kernel submission
> > +   ====================================
> > +
> > +The subsystem maintainer, when preparing the kernel patches to be sent
> > +to mainstream (or to -mm series), uses some scripts and a manual
> > +procedure, based on a quilt-like procedure, where a patch series file is
> > +generated, and patches can be handles one by one. It means that -git
>
>                                 ^handled           ^This
>
> > +patches can be properly fixed, when required, if not already sent to
> > +mainstream, to futhfill the best practices and resolve conflicts with
>
>                  ^fulfill
>
> > +other patches direclty merged in mainstream.
                  ^directly

> > +
> > +7. Patch submission from the community
> > +   ===================================
> > +
> > +Patch submission is open to all Free Software community. The general
                                    ^the
> > +procedure to have a patch accepted at v4l/dvb subsystem and at kernel is
                                       ^in the                  ^in the

> > +b. Use [PATCH] and a brief description at email's subject.
                                           ^in the

> > +   driver maintainer will apply on his tree, asking the subsystem
                                    ^to           ^and at seom later time
> > +   maintainer to pull, after some time.
        ask the subsystem maintainer to pull.

> > +g. If it is a newer driver (not yet on one of the development trees),
                                        ^in



More information about the linux-dvb mailing list