[Sugar-devel] Bug tracking Vs Patch review

[Bernie Innocenti]

El Wed, 01-09-2010 a las 23:43 +0100, Marco Pesenti Gritti escribió:

> We agree that we should try out reviews on the mailing list, let's
> just do it.

If Tomeu is ok with it, I'll repost some of my old patches to the list
to get them reviewed and, hopefully, approved.

To recap, the submission rules I propose are really simple:

1) On the submitter end:

    git commit -s
    git format-patch -1
    git send-email --to maintainer --cc list foo.patch

2) Anyone who sees the patch can reply with inline comments
   Multiple reviews are welcome.

3) The reviewer can conclude the message with one of these tags:

    Acked-by: Joe Hacker <jhacker at sugarlabs.org>
    Reviewed-by: Joe Hacker <jhacker at sugarlabs.org>
    Tested-by: Joe Hacker <jhacker at sugarlabs.org>

   Only module maintainers and peers can ack a patch.
   The meaning of these tags should be already self-explanatory.
   In case someone has doubts, here's the full explanation:
   http://lxr.linux.no/linux/Documentation/SubmittingPatches

4) if submitter has write access to the repository, he/she amends
   the patch, appending any collected tags to it:

     git commit --amend
     git push

   (submitters with multiple patches in their queue may need to
   rebase or switch to a clean branch)

5) in case the patch closes a bug in Trac, submitter closes the
   bug mentioning the commit ID as usual

Let's get started this way. If needed, we could refine the rules later
on. To avoid confusion, I'd wait updating the documentation in the wiki
until we've tested this new workflow for a while.

If a maintainer cannot stand to approve patches submitted to the
mailing-list, I'd ask them to state it clearly, so we don't needlessly
disappoint submitters. If a submitter still prefers the old workflow,
they can keep filing patches in the bug tracker as before.

Agreed?


> I'm pretty confident we can setup and improve patchwork to help us
> tracking patch status reliably. I don't have a lot of time but I will
> commit to help out with both infrastructure and the reviews
> themselves.

We've already had Patchwork on this list for a while:

  http://patchwork.sugarlabs.org/project/sugar/list/

It's a useful aid on the side, but I don't think it needs to get in the
middle of the patch workflow. People are generally good at keeping track
of threads in mailing list within their MUA.

In case a patch gets overlooked by the maintainer, the submitter can
resend it after a while. If even the submitter forgets, someone else
could ping. If nobody cares to ping, it means that patch wasn't very
interesting after all.

-- 
   // Bernie Innocenti - http://codewiz.org/
 \X/  Sugar Labs       - http://sugarlabs.org/