[oe] [RFC] Perfect(?) workflow for patches.

[Paul Menzel]

Dear OE users,


I hope this is the my last message to this list for tonight.

In the last few days good discussion was going on this list about
patches, their review and committing them. That it will not be
forgotten, I try to outline the workflow and after discussion I would
like to put it into the Wiki. Any suggestion for the name for this page?
»Patch workflow«?

Git hooks [4] were not mentioned in the discussion on the list. I added
them anyways. ;-) It is important to note that every clone has its own
hooks. The hooks can be put under revision control but they have to be
manually included in the Git setup.

So here we go.

1. A makes a change to OE and wants to get it included.
  a) A should use Git.
  b) Adhere to commit policy [1], styleguide [2] (tool (`oe-stylize.py`)
available [3]), rebase against remote branch.
  c) Send the patch to the mailing list. Preferred way is `git
send-email`. Sending it as an attachment. Additionally Git tree can be
publishing on the net.

> b) could be automated using Git hooks checking for Signed-off-by-line
> (although that can be added later in `git format-patch`), running
> `oe-stylize.py`) and check for example for spelling mistakes [5].

2. Thinks done by OE infrastructure.
  a) Message sent to the list.
  b) Patchwork is picking the patch up.
  c) If this patch is an iteration (see 5 a)), Patchwork should detect
this and mark the old patch as »Superseded«.

> I do not know if this is possible. I will ask the Patchwork hackers
> about it. If not this could be done using a Git hook at least when the
> final patch is pushed by someone with commit rights (see 4).

  d) If b) is possible it could be tried to commit the patch
automatically to a staging branch.
    + If the patch applies could be automatically tested.
      Failure: Automatically (`pwclient` [6]) change the state in
Patchwork to »Does not apply« and sent message to list (including
author).
    - Security implications. Evil people could for example sent a patch
too big. ikiwiki offers something similar [7].

3. Review
  a) Reviewing without applying. Answering directly to the list and
change the state in Patchwork.

> A way is missing to easily retrieve the Patchwork ID.

  b) Reviewer get the patch either using saving the message sent with
`git send-email`, save the attachment, retrieve it from Patchwork
(`pwclient`, `pw-am.sh` [8]) and (if not happened already) apply it
using `git am file`. A pre-applypatch hook checks again for the things
in 1. b) and a post-applypatch hook sets the patchwork state to »Under
Review«. After review a reply is sent back to the list and the Patchwork
state changed appropriately. A »Reviewed-by« or »Acked-by« line can be
added. (Or just »Signed-off-by«?)

> I have not found out yet, if the hooks can be made dependent on a
> branch, so that applying the patch to a branch (i. e. accepted) can
> trigger a certain hook. If not it should be scriptable to set the
> appropriate state (»Accepted«, »Rejected«) [9].

     If the reviewer has a question to the patch should the Patchwork
state be changed to »Deferred«?

4. Commit (if not go to 5.)
  a) If the reviewer has commit access and accepts the patch, s/he can
push it. A hook then changes the Patchwork state to »Applied« with the
appropriate commit id and archived(?).

     Furthermore should a message be send to the list that the patch has
been committed?

5. Reject.
  a) The original author (or someone) else incorporates the comments and
publishes a new patch with its iteration in the subject `[PATCH v4]`
(`git format-patch --subject-prefix=`PATCH v4`) and maybe a short
description of the changes of the iteration in the commit message.
  b) Go to 1..


Sorry for this long message. Maybe a schematic diagram should be made
out of this. I hope figuring this out will improve OE’s code quality a
bit and be a good reference to point new developers to.


Thanks,

Paul


[1] http://wiki.openembedded.net/index.php/Commit_Policy
[2] http://wiki.openembedded.net/index.php/Styleguide
[3] http://wiki.openembedded.net/index.php/Styleguide#Style_Checking_tools
[4] http://benjamin-meyer.blogspot.com/2008/10/git-hooks.html
[5] http://github.com/Arora/arora/tree/master/git_hooks/
[6] http://patchwork.openembedded.org/help/pwclient/
[7] http://ikiwiki.info/tips/untrusted_git_push/
[8] http://cgit.openembedded.org/cgit.cgi/openembedded/tree/contrib/patchwork/pw-am.sh
[9] http://lists.ozlabs.org/pipermail/patchwork/attachments/20091006/13e2340a/attachment.asc
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Dies ist ein digital signierter Nachrichtenteil
URL: <http://lists.openembedded.org/pipermail/openembedded-devel/attachments/20100124/8f8f4823/attachment-0002.sig>