[Sugar-devel] review process changes

[Tomeu Vizoso]

On Thu, May 27, 2010 at 08:44, James Cameron <quozl at laptop.org> wrote:
> On Wed, May 26, 2010 at 01:48:17PM +0200, Tomeu Vizoso wrote:
>> == Changes I would like to see ==
>>
>> * Having _all_ reviews in the mailing list. The process already allows
>> patches to be sent and discussed in the mailing list, but restricts it
>> to "new feature[s] and reasonably big".
>
> It also says under 2000 lines does not need breaking into separate
> commits.  ;-)  I think it is not good to set such limits, instead rely
> on feedback during review.

Sure, it's just an attempt to save a round trip, in general I think
patches with less than 2000 lines would benefit from splitting, but
line count is not enough to tell.

>> * Reduce the time spent creating and changing tickets in Trac
>
> A command line interface would be nice.  I'm on a high latency
> connection, working from outback Australia.

Yes, an xml-rpc extension was installed in trac to allow for this, but
I haven't heard any progress on it. I use git-bz for both freedesktop
and gnome and I rarely need to use the web interface:

http://git.fishsoup.net/man/git-bz.html

>> == Issues I need help with ==
>>
>> * How can a maintainer keep track of his queue with something like
>> http://bugs.sugarlabs.org/wiki/TomeuReviewQueue if patches are sent to
>> the list?
>
> Bernie has suggested Patchwork.  I've investigated it some more ...
>
> Initial configuration:
>
> You should do this; visit http://patchwork.sugarlabs.org/ ... register
> ... wait for e-mail ...  login ... tell Bernie or myself your username,
> we shall add you as a "maintainer" of "sugar" using the admin interface
> ... until then your username is not available in the list of delegate
> targets.
>
> Ongoing usage ... triage:
>
> You should do this; http://patchwork.sugarlabs.org/project/sugar/list/
> ... for patches that are New, click on them, delegate to a maintainer,
> can be done by you or by other users of patchwork.
>
> Ongoing usage ... maintainer todo list:
>
> You should do this; http://patchwork.sugarlabs.org/user/todo/ ...
> displays the patches that the maintainer has in their TODO list.
>
> Things to note:
>
> - "maintainer" in Patchwork means you have the right to approve patches
>  for a "project".
>
> - temporarily Patchwork instance has all user profiles marked as
>  "maintainer", but this should change later.
>
> - other successful projects using Patchwork typically have a one to one
>  mapping between mailing list and git repository.  We don't have that.

Created an account, I guess we can make each module in
http://wiki.sugarlabs.org/go/Development_Team/Release/Modules a
project with their own maintainers?

>> * How can we make sure that patches don't fall through the cracks?
>
> That remains a problem with any process, and was a problem with the
> existing process, and the Patchwork tool should provide a partial
> control.

Sure, the question was intended as a starting place for comparing the
different alternatives and see how they fare in not letting patches
forgotten.

> Patchwork will help maintainers track the state of a patch external to
> mail applications.
>
>> * How can I know if the patch I'm staring at is the latest version?
>
> Check for later mail by same author (if they did not post mail, that's
> their fault), check for a repository or branch offer.

This gets messy very quickly and would be a sad regression from the
existing system. Patchwork's queue doesn't display what is thought to
be the patch awaiting review? If so, submitters could check by
themselves that the reviewer is going to look at the correct patch.

>> * How can we make easy to go from git blame to the review thread, the
>> bug report and the feature page?
>
> Search your mail for the patch hash and display the results as a thread?
> I use mairix for this, with mutt as the display engine.

That sounds like another regression to me, cannot do better than that?

Thanks a lot for your comments. What about the other patch submitters?
Are they happy with the system in place?

Regards,

Tomeu

> --
> James Cameron
> http://quozl.linux.org.au/
>