[TYPO3-core] Approving patches

Michael Stucki michael at typo3.org
Mon Oct 30 12:50:35 CET 2006


Hi Kasper,

> In a separate thread, how can we address the problem of "+1"s not given?
> 
> On a short term (for 4.1) what can we do? Could everyone please give
> their reasons to not respond or maybe post your commitment to review
> some patches?

I find it bad that we have problems with the review process, because I think
that the four-eye-review procedure is the right way. Quality feedbacks seem
to prove this. We have found the right procedure, but now we need to tune
it a little bit.

First, I feel kind of responsible to review patches when nobody else is
doing it. However, I did not have much time to do that in the past few
weeks and months. Sorry for this, I'm trying to improve it.

But I also see it as a problem that only a few of us are actively doing
reviews. Maybe(?) reviewing should become a requirement for retain core
team membership?

Another idea that just comes to my mind is that we could split up the review
process (at least for bugs) into a) problem and b) solution:

- Problem:
  Someone who reports a bug can approve that the patch solves the problem

- Solution:
  Only one more review from a core team member is needed, just to approve
  that it is done right (e.g. check if the fix is at the right place,
  checking the coding style, documentation is provided, etc.)


Last but not least, we should think about inviting some people to this list
just for reviewing patches. I don't see too much fun in this, but probably
there are some people willing to do that for us...(?)

> On a long term my plan is this:
> 
> - All patches (for bugs or features) go through the bug-tracker

Good.

> - Anyone can post a patch there
> - A group of people larger than today are allowed to give "+1"s to
> patches. This group is made up by recommendations coming from us (fx.
> Ingmar recommends Batman who recommends Robin and Pinguin, Pinguin
> recommends catwoman and her 5 girlfriends. They are all allowed to
> give "+1"s.). Recommendations happen through software so we can
> always track the chain. Ingmar can cut of Batman if it turns out that
> Catwoman and her friends poohs in the source - so in this way there
> is always a reflections of bad work back in the chain.
> - A patch still requires X number of "+1"s to get into the core.
> These are given through the bug-tracker in the future, not the
> mailing list (so we can track who approved something).

We discussed that in Karlsruhe already: I'm not sure if this is the right
solution. I fear that it will add a lot more work for us. The core list
would become obsolete. People will still need to find someone who reviews a
patch. After all, I expect that the overall quality is going to fall.

Just to give you an example: I still read every patch just to see if there
is something documentation-related inside, because that workflow didn't
work for several times. How should this work better when even more people
send their patches over a decentralized channel?

Besides of all this, I think that the technical implementation of such a
procedure (requires changes to Mantis bugtracker, for example) is a lot of
work which could be spent better than for an experiment like this. After
all we cannot estimate how good it will work in the end...

> - A smaller group has SVN access and their job is to blindly commit
> the patches as they have reached enough "+1"s. The responsibility of
> the committer is to make sure he doesn't mess up SVN and that only
> "+1"-approved patches goes there. It is not his responsibility if the
> patch was bad - that will reflect back on those who approved it!

And in case the stuff was bad: Who is going to take care of reverting these
things? I just remember that I once spent more than 4 hours just to revert
Renés MM-handling improvements.

> Inside this system there are karma-based incentives; People get
> points for approving, committing etc. and can equally loose points if
> they make shit (and others can earn points if they fix the shit!).

Again: If someone approves a patch but actually didn't test anything, it
will be very difficult to revert that stuff. Plus: Every change which he
approved would have to be put in question.

> What I hope to obtain is a more automated approach to core
> contributions but still keeping control. If we can finetune this
> system to fit reality it will balance in itself. People can get more
> or less active in periods without affecting our overall output
> because the crowd of "patch-approvers" are in the hundreds, not tens.

I like the overall approach of giving more people the possibility to
contribute to the core. However, I think that a snow-ball system for
permitting operations is a bad way of doing this.

- michael
-- 
Use a newsreader! Check out
http://typo3.org/community/mailing-lists/use-a-news-reader/



More information about the TYPO3-team-core mailing list