[Sugar-devel] Bug tracking Vs Patch review

[Tomeu Vizoso]

On Thu, Sep 2, 2010 at 22:02, Bernie Innocenti <bernie at codewiz.org> wrote:
> 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?

Fine with me, though if people could wait a couple of weeks before
sending stuff only for 0.92 it would save quite a bit of time by not
having to branch and cherry pick stuff into sucrose-0.90 so early.

>> 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.

I personally agree with this, but some people in this project seem to
have problem with pinging. Maybe the change of process will be a good
chance for people to lose their timidness, we'll see.

Regards,

Tomeu

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