Re: [Sugar-devel] Proposal on how to speed up patch reviews

[Simon Schampijer]

On 03/25/2013 01:27 PM, Gonzalo Odiard wrote:

While I agree in theory with all this,
we can improve our actual situation if we look at our resources,
time and people.

* Time: we don't have a schedule, then feature discussion can't start.
We can improve if we have a clear path and start to define what features
will be included in the next cycle. I am sure different players
right now have different ideas of what the next cycle can/should be,
we need a agreement and try to push together.


Yes, having a schedule will help. I guess if there is not a good reason 
to switch to feature based releases we can stick with time based ones. 
And keep on following the GNOME/Fedora schedule that would be 
September/October again. I guess your brain can start to think about 
what you want to achieve in such a time frame...(yes we should make a 
general call for this).



* People: We don't have enough people really,
and this situation can be temporally more difficult if some maintainer
have by example  a baby :) (to not use the classic "Linus gets hit by a
bus")
As a project, we need take care of the people working with us,
see at the personal situations, and the maintainers should
open the door to more people to be involved if possible.
For a long time, we had only Simon and Sascha doing sugar maintainership,
and was not enough, now we have Simon and Manuq,
and there are too much work to do. I think we should continue
work on trying to get somebody else involved as maintainer.

Gonzalo


If you see people around that want to be maintainers and feel ready for 
it, great. I think I have not pushed back on someone who stepped up so 
far :)


But I think even if you aren't a nominated maintainer you can help the 
current maintainers with the work load. As described in this thread 
people can help with review and testing and helping with product polish 
especially in the last part of a cycle is helpful as well.


Cheers,
   Simon
___
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel

Re: [Sugar-devel] Proposal on how to speed up patch reviews

[Daniel Narvaez]

On 25 March 2013 13:27, Gonzalo Odiard  wrote:
> While I agree in theory with all this,
> we can improve our actual situation if we look at our resources,
> time and people.
>
> * Time: we don't have a schedule, then feature discussion can't start.
> We can improve if we have a clear path and start to define what features
> will be included in the next cycle. I am sure different players
> right now have different ideas of what the next cycle can/should be,
> we need a agreement and try to push together.

Absolutely. I think finding consensus on a focus for the next release
is the top priority now. It's a discussion we should start, possibly
involving IAEP in it.

> * People: We don't have enough people really,
> and this situation can be temporally more difficult if some maintainer
> have by example  a baby :) (to not use the classic "Linus gets hit by a
> bus")
> As a project, we need take care of the people working with us,
> see at the personal situations, and the maintainers should
> open the door to more people to be involved if possible.
> For a long time, we had only Simon and Sascha doing sugar maintainership,
> and was not enough, now we have Simon and Manuq,
> and there are too much work to do. I think we should continue
> work on trying to get somebody else involved as maintainer.

The main goal of my proposal is exactly to open door to more people. I
think it's hard to find maintainers both because of the time
commitment and the experience required. We should keep looking but I
suspect we won't find them overnight. It's much easier to find
reviewers, I actually can think of several people in the community
that could cover that role very well already.
___
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel

Re: [Sugar-devel] Proposal on how to speed up patch reviews

[Gonzalo Odiard]

While I agree in theory with all this,
we can improve our actual situation if we look at our resources,
time and people.

* Time: we don't have a schedule, then feature discussion can't start.
We can improve if we have a clear path and start to define what features
will be included in the next cycle. I am sure different players
right now have different ideas of what the next cycle can/should be,
we need a agreement and try to push together.

* People: We don't have enough people really,
and this situation can be temporally more difficult if some maintainer
have by example  a baby :) (to not use the classic "Linus gets hit by a
bus")
As a project, we need take care of the people working with us,
see at the personal situations, and the maintainers should
open the door to more people to be involved if possible.
For a long time, we had only Simon and Sascha doing sugar maintainership,
and was not enough, now we have Simon and Manuq,
and there are too much work to do. I think we should continue
work on trying to get somebody else involved as maintainer.

Gonzalo

On Mon, Mar 25, 2013 at 8:37 AM, Simon Schampijer wrote:

> Hey Daniel,
>
> thanks for the write-up!
>
> It is true that patch review, especially of non-bugfix patches is slow and
> can be sometimes frustrating for all parties involved, the reviewer, the
> maintainer and the submitter.
>
> To improve that situation, I think we have to put some lights on all those
> roles and I think often the situation of the maintainer is not understood
> well enough. I can at least say that we had this discussions for the last
> years in Sugar, with different maintainers and I have seen it happening in
> many other projects as well. It is a known issue, and it is not an easy
> one, never less I think we can improve.
>
> I think there are different kind of patches, let's simplify with 4
> categories:
>
> [bugfix] a bugfix
>
> [feature] a Feature
>
> [feature/UI] a Feature with UI
>
> [cleanup] a cleanup/maintenance
>
> There are different kind of roles in the process and each will act
> differently based on his needs and duties. There are the submitters, the
> reviewers (can be either the maintainer, or someone else from the
> community) and the maintainers. Let's step through the categories and talk
> about the different roles:
>
> [bugfix] A bugfix is the simplest case. The submitter is interested in
> solving the specific issue he is working on. It is not hard to find a
> reviewer or tester. Either someone from the community that has been annoyed
> by the same issue or the maintainer himself because he is interested in
> seeing the issue solved, his interest is a working code base in the end.
> Here it is easy as well for a maintainer to trust another reviewer and
> acknowledge based on his positive review.
>
> In my experience those patches are not laying around for long if there is
> not a technical blocker in the patch itself. Evaluation is relatively easy
> here.
>
>
> [feature] When it gets to Features things get more tricky. For a Feature
> first of all the high level goals are important: what need does the Feature
> address, is it wanted by the community, is the technical approach taken a
> good one, basically the maintainer has to decide if it is worth taking on
> maintainership of this feature or not. In the end it might be him who has
> to deal with arising bug fixes and who is blamed if the software is not a
> solid product.
>
> That might explain a general bigger resistance to features by maintainers.
> How can we help each other to make this a better process?
>
> First of all, it is important to know the high level goals of a feature.
> The feature process [1] in place actually helps in that regard, the
> maintainer can do the evaluation based on the community feedback and the
> high level goals and if those are accepted can do the actual review of the
> code.
>
> Getting approval for a feature does take time, and the review itself often
> as well, this should be counted in as well from all the sides, especially
> when it is the desire to see a specific feature being present in a release.
>
> Setting goals at the beginning of a release and having an accepting
> feature period will help here as well, actually all of this is in place
> already.
>
>
> [feature/UI] All what have been said above applies to the feature that
> adds UI as well. I have separated that item because it adds another
> complexity. We have the UI process for this (as written in the feature
> policy [1]). Basically it adds more care taking of all the roles involved.
>
>
> [cleanup] These are simple as well and can be acknowledged without much
> thinking. In my opinion, maintainers should be able to do those without the
> need of a review, if uncertain or bigger parts are touched they can discuss
> the items with the other maintainers.
>
>
> [1] 
> http://wiki.sugarlabs.org/go/**Features/Policy
>
>
>
> On 03/24/2013 08:30 PM, Daniel Nar

Re: [Sugar-devel] Proposal on how to speed up patch reviews

[Simon Schampijer]

On 03/25/2013 12:46 PM, Walter Bender wrote:

On Mon, Mar 25, 2013 at 7:37 AM, Simon Schampijer  wrote:

Hey Daniel,

thanks for the write-up!

It is true that patch review, especially of non-bugfix patches is slow and
can be sometimes frustrating for all parties involved, the reviewer, the
maintainer and the submitter.

To improve that situation, I think we have to put some lights on all those
roles and I think often the situation of the maintainer is not understood
well enough. I can at least say that we had this discussions for the last
years in Sugar, with different maintainers and I have seen it happening in
many other projects as well. It is a known issue, and it is not an easy one,
never less I think we can improve.

I think there are different kind of patches, let's simplify with 4
categories:

[bugfix] a bugfix

[feature] a Feature

[feature/UI] a Feature with UI

[cleanup] a cleanup/maintenance

There are different kind of roles in the process and each will act
differently based on his needs and duties. There are the submitters, the
reviewers (can be either the maintainer, or someone else from the community)
and the maintainers. Let's step through the categories and talk about the
different roles:

[bugfix] A bugfix is the simplest case. The submitter is interested in
solving the specific issue he is working on. It is not hard to find a
reviewer or tester. Either someone from the community that has been annoyed
by the same issue or the maintainer himself because he is interested in
seeing the issue solved, his interest is a working code base in the end.
Here it is easy as well for a maintainer to trust another reviewer and
acknowledge based on his positive review.

In my experience those patches are not laying around for long if there is
not a technical blocker in the patch itself. Evaluation is relatively easy
here.


[feature] When it gets to Features things get more tricky. For a Feature
first of all the high level goals are important: what need does the Feature
address, is it wanted by the community, is the technical approach taken a
good one, basically the maintainer has to decide if it is worth taking on
maintainership of this feature or not. In the end it might be him who has to
deal with arising bug fixes and who is blamed if the software is not a solid
product.

That might explain a general bigger resistance to features by maintainers.
How can we help each other to make this a better process?

First of all, it is important to know the high level goals of a feature. The
feature process [1] in place actually helps in that regard, the maintainer
can do the evaluation based on the community feedback and the high level
goals and if those are accepted can do the actual review of the code.

Getting approval for a feature does take time, and the review itself often
as well, this should be counted in as well from all the sides, especially
when it is the desire to see a specific feature being present in a release.

Setting goals at the beginning of a release and having an accepting feature
period will help here as well, actually all of this is in place already.


[feature/UI] All what have been said above applies to the feature that adds
UI as well. I have separated that item because it adds another complexity.
We have the UI process for this (as written in the feature policy [1]).
Basically it adds more care taking of all the roles involved.


[cleanup] These are simple as well and can be acknowledged without much
thinking. In my opinion, maintainers should be able to do those without the
need of a review, if uncertain or bigger parts are touched they can discuss
the items with the other maintainers.


[1] http://wiki.sugarlabs.org/go/Features/Policy



On 03/24/2013 08:30 PM, Daniel Narvaez wrote:


Hello,

I think patch reviews are taking too long. It's probably a well known
issue but I will give a couple of examples as a "proof".

Online services patches, unreviewed  for more than one month
http://lists.sugarlabs.org/archive/sugar-devel/2013-February/041808.html
Unreviewed for more than one month



This one is part of the [feature] category. It can be mostly explained with
the maintainers having their heads still in bug fixing for 0.98.

Here applies as well what I said with high level descriptions of features
and the feature process [2].

[2] http://lists.sugarlabs.org/archive/sugar-devel/2013-February/041810.html


For the record, [3], [4] were sent to the devel list within 6 hours of
your request [2] :P


Ahh, excellent. Didn't see them. In any case, was just an example of a 
typical case.


Regards,
   Simon

___
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel

Re: [Sugar-devel] Proposal on how to speed up patch reviews

[Walter Bender]

On Mon, Mar 25, 2013 at 7:37 AM, Simon Schampijer  wrote:
> Hey Daniel,
>
> thanks for the write-up!
>
> It is true that patch review, especially of non-bugfix patches is slow and
> can be sometimes frustrating for all parties involved, the reviewer, the
> maintainer and the submitter.
>
> To improve that situation, I think we have to put some lights on all those
> roles and I think often the situation of the maintainer is not understood
> well enough. I can at least say that we had this discussions for the last
> years in Sugar, with different maintainers and I have seen it happening in
> many other projects as well. It is a known issue, and it is not an easy one,
> never less I think we can improve.
>
> I think there are different kind of patches, let's simplify with 4
> categories:
>
> [bugfix] a bugfix
>
> [feature] a Feature
>
> [feature/UI] a Feature with UI
>
> [cleanup] a cleanup/maintenance
>
> There are different kind of roles in the process and each will act
> differently based on his needs and duties. There are the submitters, the
> reviewers (can be either the maintainer, or someone else from the community)
> and the maintainers. Let's step through the categories and talk about the
> different roles:
>
> [bugfix] A bugfix is the simplest case. The submitter is interested in
> solving the specific issue he is working on. It is not hard to find a
> reviewer or tester. Either someone from the community that has been annoyed
> by the same issue or the maintainer himself because he is interested in
> seeing the issue solved, his interest is a working code base in the end.
> Here it is easy as well for a maintainer to trust another reviewer and
> acknowledge based on his positive review.
>
> In my experience those patches are not laying around for long if there is
> not a technical blocker in the patch itself. Evaluation is relatively easy
> here.
>
>
> [feature] When it gets to Features things get more tricky. For a Feature
> first of all the high level goals are important: what need does the Feature
> address, is it wanted by the community, is the technical approach taken a
> good one, basically the maintainer has to decide if it is worth taking on
> maintainership of this feature or not. In the end it might be him who has to
> deal with arising bug fixes and who is blamed if the software is not a solid
> product.
>
> That might explain a general bigger resistance to features by maintainers.
> How can we help each other to make this a better process?
>
> First of all, it is important to know the high level goals of a feature. The
> feature process [1] in place actually helps in that regard, the maintainer
> can do the evaluation based on the community feedback and the high level
> goals and if those are accepted can do the actual review of the code.
>
> Getting approval for a feature does take time, and the review itself often
> as well, this should be counted in as well from all the sides, especially
> when it is the desire to see a specific feature being present in a release.
>
> Setting goals at the beginning of a release and having an accepting feature
> period will help here as well, actually all of this is in place already.
>
>
> [feature/UI] All what have been said above applies to the feature that adds
> UI as well. I have separated that item because it adds another complexity.
> We have the UI process for this (as written in the feature policy [1]).
> Basically it adds more care taking of all the roles involved.
>
>
> [cleanup] These are simple as well and can be acknowledged without much
> thinking. In my opinion, maintainers should be able to do those without the
> need of a review, if uncertain or bigger parts are touched they can discuss
> the items with the other maintainers.
>
>
> [1] http://wiki.sugarlabs.org/go/Features/Policy
>
>
>
> On 03/24/2013 08:30 PM, Daniel Narvaez wrote:
>>
>> Hello,
>>
>> I think patch reviews are taking too long. It's probably a well known
>> issue but I will give a couple of examples as a "proof".
>>
>> Online services patches, unreviewed  for more than one month
>> http://lists.sugarlabs.org/archive/sugar-devel/2013-February/041808.html
>> Unreviewed for more than one month
>
>
> This one is part of the [feature] category. It can be mostly explained with
> the maintainers having their heads still in bug fixing for 0.98.
>
> Here applies as well what I said with high level descriptions of features
> and the feature process [2].
>
> [2] http://lists.sugarlabs.org/archive/sugar-devel/2013-February/041810.html

For the record, [3], [4] were sent to the devel list within 6 hours of
your request [2] :P

[3] http://lists.sugarlabs.org/archive/sugar-devel/2013-February/041817.html
[4] http://lists.sugarlabs.org/archive/sugar-devel/2013-February/041820.html

>
>
>> Various patches, including automated testing stuff
>> http://lists.sugarlabs.org/archive/sugar-devel/2012-December/041218.html
>> Unreviewed for more than three months
>
>
> This is nearly category 

Re: [Sugar-devel] Proposal on how to speed up patch reviews

[Simon Schampijer]

Hey Daniel,

thanks for the write-up!

It is true that patch review, especially of non-bugfix patches is slow 
and can be sometimes frustrating for all parties involved, the reviewer, 
the maintainer and the submitter.


To improve that situation, I think we have to put some lights on all 
those roles and I think often the situation of the maintainer is not 
understood well enough. I can at least say that we had this discussions 
for the last years in Sugar, with different maintainers and I have seen 
it happening in many other projects as well. It is a known issue, and it 
is not an easy one, never less I think we can improve.


I think there are different kind of patches, let's simplify with 4 
categories:


[bugfix] a bugfix

[feature] a Feature

[feature/UI] a Feature with UI

[cleanup] a cleanup/maintenance

There are different kind of roles in the process and each will act 
differently based on his needs and duties. There are the submitters, the 
reviewers (can be either the maintainer, or someone else from the 
community) and the maintainers. Let's step through the categories and 
talk about the different roles:


[bugfix] A bugfix is the simplest case. The submitter is interested in 
solving the specific issue he is working on. It is not hard to find a 
reviewer or tester. Either someone from the community that has been 
annoyed by the same issue or the maintainer himself because he is 
interested in seeing the issue solved, his interest is a working code 
base in the end. Here it is easy as well for a maintainer to trust 
another reviewer and acknowledge based on his positive review.


In my experience those patches are not laying around for long if there 
is not a technical blocker in the patch itself. Evaluation is relatively 
easy here.



[feature] When it gets to Features things get more tricky. For a Feature 
first of all the high level goals are important: what need does the 
Feature address, is it wanted by the community, is the technical 
approach taken a good one, basically the maintainer has to decide if it 
is worth taking on maintainership of this feature or not. In the end it 
might be him who has to deal with arising bug fixes and who is blamed if 
the software is not a solid product.


That might explain a general bigger resistance to features by 
maintainers. How can we help each other to make this a better process?


First of all, it is important to know the high level goals of a feature. 
The feature process [1] in place actually helps in that regard, the 
maintainer can do the evaluation based on the community feedback and the 
high level goals and if those are accepted can do the actual review of 
the code.


Getting approval for a feature does take time, and the review itself 
often as well, this should be counted in as well from all the sides, 
especially when it is the desire to see a specific feature being present 
in a release.


Setting goals at the beginning of a release and having an accepting 
feature period will help here as well, actually all of this is in place 
already.



[feature/UI] All what have been said above applies to the feature that 
adds UI as well. I have separated that item because it adds another 
complexity. We have the UI process for this (as written in the feature 
policy [1]). Basically it adds more care taking of all the roles involved.



[cleanup] These are simple as well and can be acknowledged without much 
thinking. In my opinion, maintainers should be able to do those without 
the need of a review, if uncertain or bigger parts are touched they can 
discuss the items with the other maintainers.



[1] http://wiki.sugarlabs.org/go/Features/Policy


On 03/24/2013 08:30 PM, Daniel Narvaez wrote:

Hello,

I think patch reviews are taking too long. It's probably a well known
issue but I will give a couple of examples as a "proof".

Online services patches, unreviewed  for more than one month
http://lists.sugarlabs.org/archive/sugar-devel/2013-February/041808.html
Unreviewed for more than one month


This one is part of the [feature] category. It can be mostly explained 
with the maintainers having their heads still in bug fixing for 0.98.


Here applies as well what I said with high level descriptions of 
features and the feature process [2].


[2] http://lists.sugarlabs.org/archive/sugar-devel/2013-February/041810.html


Various patches, including automated testing stuff
http://lists.sugarlabs.org/archive/sugar-devel/2012-December/041218.html
Unreviewed for more than three months


This is nearly category [cleanup]. We should not block here on a in 
depth review when things pass the usual tests (pep8 etc) and the high 
level approach is accepted (taking for granted that the maintainer is 
fixing up the issues that might arise himself).



I think this situation is seriously hurting the project, in several ways:


Agreed.


* It makes hacking on sugar not fun at all. I honestly even lost
interest in the patches I sent at this point.
* It discou

Re: [Sugar-devel] Proposal on how to speed up patch reviews

[Walter Bender]

On Sun, Mar 24, 2013 at 3:30 PM, Daniel Narvaez  wrote:
> Hello,
>
> I think patch reviews are taking too long. It's probably a well known
> issue but I will give a couple of examples as a "proof".
>
> Online services patches, unreviewed  for more than one month
> http://lists.sugarlabs.org/archive/sugar-devel/2013-February/041808.html
> Unreviewed for more than one month
>
> Various patches, including automated testing stuff
> http://lists.sugarlabs.org/archive/sugar-devel/2012-December/041218.html
> Unreviewed for more than three months
>
> I think this situation is seriously hurting the project, in several ways:
>
> * It makes hacking on sugar not fun at all. I honestly even lost
> interest in the patches I sent at this point.
> * It discourages new contributors. I know maintainers care about the
> patches I've been mentioning and I'm pretty confident they are good
> enough to be considered if not merged. It's just matter of priorities
> and lack of time. But less experienced contributors might think the
> project doesn't care about their contributions or they are not
> considered even worth an answer.
> * It keeps out the the tree code which could benefit the project. If
> my automated tests patches was in perhaps we could ask people to write
> tests for their code in reviews. If online services patches was in
> perhaps other people could build cool stuff on the top of that API.
> * It slows down the development peace. I reallly think active
> development brings more development because it makes the project more
> attractive to hackers.
> * By discouraging people to contribute we make it impossible to get
> more "maintainers" on board, which of course makes the problem only
> worst.
> * It blocks people which are persisent enough to not get discouraged.
> I don't know how many times I holded back writing a patch because I
> didn't want to add of my 50+ queue of patches to be reviewed. It's
> just too much of an hassle to manage a lot of new code outside of the
> tree for long.
>
> Here is my proposal to try to improve the situation.
>
> * Let's separate the maintainer from the reviewer roles. Maintainers
> should be very expert because ultimately the future of the project is
> in their hands. Those kind of people are very rare. Though I think
> there are many more people that could do reviews on areas of the code
> they feel comfortable with.
> * The final word would still all be in the hands of maintainers. If
> they don't like a patch they override reviewers opinion. If they want
> more time to look at the patch or someone else to take a look, they
> can just ask that. They also are the ones that choose reviewers. But
> if a reviewer approves a patch and maintainers don't complain, then
> the patch lands.
> * By having not-yet-super-experts doing reviews we are maybe going to
> lower quality a bit. But there are other ways to ensure quality that
> doesn't slow down development as much as depending on a very small
> group of people to review any code change. Automated code checking
> (pep8 and pyflakes are very useful and easy to setup), automated
> testing, human testing. Honestly our attempt to ensure quality at
> review time is not even being very successful, we have a lot of more
> bugs than we should have.
>
> So more in concrete what I'm proposing is:
>
> * We nominate Manuel and Simon maintainers of glucose as a whole. Just
> a cleanup while we are making changes... They are maintainers of all
> the modules anyway, it doesn't make a lot of sense.
> * Maintainers makes a (long enough!) list of reviewers and update it
> when new "experts" shows up. I don't think they need to per module,
> they can just review what they are comfortable to. The reviewers
> approve and, if necessary, land patches.
> * We go back requiring all the patches to be posted on the mailing
> list (that's still the officlal policy but in practice a lot of code
> is going through trac these days). This is necessary so that
> maintainers can see all that is being reviewed and can comment if
> required.
> * We setup automated pep8 and pyflakes for all the modules. We start
> pretending unit/integration tests to go with patches whenever it's
> possible (UI bits are going to require some more infrastructure before
> they are possible, we can get there gradually). We start getting some
> human testing on master.
>
> Hope it helps.
>
> --
> Daniel Narvaez
> ___
> Sugar-devel mailing list
> Sugar-devel@lists.sugarlabs.org
> http://lists.sugarlabs.org/listinfo/sugar-devel

All seems reasonable to me.

-walter

-- 
Walter Bender
Sugar Labs
http://www.sugarlabs.org
___
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel

[Sugar-devel] Proposal on how to speed up patch reviews

[Daniel Narvaez]

Hello,

I think patch reviews are taking too long. It's probably a well known
issue but I will give a couple of examples as a "proof".

Online services patches, unreviewed  for more than one month
http://lists.sugarlabs.org/archive/sugar-devel/2013-February/041808.html
Unreviewed for more than one month

Various patches, including automated testing stuff
http://lists.sugarlabs.org/archive/sugar-devel/2012-December/041218.html
Unreviewed for more than three months

I think this situation is seriously hurting the project, in several ways:

* It makes hacking on sugar not fun at all. I honestly even lost
interest in the patches I sent at this point.
* It discourages new contributors. I know maintainers care about the
patches I've been mentioning and I'm pretty confident they are good
enough to be considered if not merged. It's just matter of priorities
and lack of time. But less experienced contributors might think the
project doesn't care about their contributions or they are not
considered even worth an answer.
* It keeps out the the tree code which could benefit the project. If
my automated tests patches was in perhaps we could ask people to write
tests for their code in reviews. If online services patches was in
perhaps other people could build cool stuff on the top of that API.
* It slows down the development peace. I reallly think active
development brings more development because it makes the project more
attractive to hackers.
* By discouraging people to contribute we make it impossible to get
more "maintainers" on board, which of course makes the problem only
worst.
* It blocks people which are persisent enough to not get discouraged.
I don't know how many times I holded back writing a patch because I
didn't want to add of my 50+ queue of patches to be reviewed. It's
just too much of an hassle to manage a lot of new code outside of the
tree for long.

Here is my proposal to try to improve the situation.

* Let's separate the maintainer from the reviewer roles. Maintainers
should be very expert because ultimately the future of the project is
in their hands. Those kind of people are very rare. Though I think
there are many more people that could do reviews on areas of the code
they feel comfortable with.
* The final word would still all be in the hands of maintainers. If
they don't like a patch they override reviewers opinion. If they want
more time to look at the patch or someone else to take a look, they
can just ask that. They also are the ones that choose reviewers. But
if a reviewer approves a patch and maintainers don't complain, then
the patch lands.
* By having not-yet-super-experts doing reviews we are maybe going to
lower quality a bit. But there are other ways to ensure quality that
doesn't slow down development as much as depending on a very small
group of people to review any code change. Automated code checking
(pep8 and pyflakes are very useful and easy to setup), automated
testing, human testing. Honestly our attempt to ensure quality at
review time is not even being very successful, we have a lot of more
bugs than we should have.

So more in concrete what I'm proposing is:

* We nominate Manuel and Simon maintainers of glucose as a whole. Just
a cleanup while we are making changes... They are maintainers of all
the modules anyway, it doesn't make a lot of sense.
* Maintainers makes a (long enough!) list of reviewers and update it
when new "experts" shows up. I don't think they need to per module,
they can just review what they are comfortable to. The reviewers
approve and, if necessary, land patches.
* We go back requiring all the patches to be posted on the mailing
list (that's still the officlal policy but in practice a lot of code
is going through trac these days). This is necessary so that
maintainers can see all that is being reviewed and can comment if
required.
* We setup automated pep8 and pyflakes for all the modules. We start
pretending unit/integration tests to go with patches whenever it's
possible (UI bits are going to require some more infrastructure before
they are possible, we can get there gradually). We start getting some
human testing on master.

Hope it helps.

-- 
Daniel Narvaez
___
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel