← Back to team overview

openerp-community team mailing list archive

Re: [slightly OT] code review at mozilla

 

Very nice article, well worth the 10 minutes to read.

About the only thing I want to add is that our current process doesn't have
specific goals of how many reviews should happen before something "may" be
merged and maybe also we need to ask the first reviewers to request
specific reviews from other people if they are knowledgeable on the subject.

Right now I try to stay out of merge requests which are outside my comfort
zone because my input there is of limited functional value, I can only
point out some nit (as the article calls them), whenever I do see some code
that interacts with code I see/use regularly I give it a more thorough look
and if I happen to have a functioning database which can easily test the
modifications in a running instance I will run that too.

Perhaps we should establish the review level as those 3 levels ? Something
like these types:
* nit
* functional
* practical

We could then ask that a merge proposal would have at least a certain
amount of reviews on each type, something like 1 practical, 2 functional, 4
nit reviews.
The numbers are fictional but indicate a certain scaling value depending on
the review "value", I'd rather have it that the nit reviews do not "grant"
access to merge the request.

Further possible approaches (not sure how feasible they are to track the
requirements) would be that instant merging is allowed given 1 practical or
2 functional reviews, if the merge proposal had only 1 functional review
for more than a week I think it would be safe to integrate the proposal as
well. This would ensure a smooth flow even if some people happen to have
some less time to do reviews with their expertise.

I'm all open for more discussion about these numbers and levels ofcourse,
just consider this a stepping stone.

Regards,
Niels


2013/1/21 Alexandre Fayolle <alexandre.fayolle@xxxxxxxxxxxxxx>

> I found the following article via Linux Weekly News, and found it very
> interesting, so I thought I might share it with the community.
>
> http://vocamus.net/dave/?p=1569
>
> Thanks for your time, and keep on sending MP. We are happy to review
> them :-)
>
> --
> Alexandre Fayolle
> Chef de Projet
> Tel : + 33 (0)4 79 26 57 94
>
> Camptocamp France SAS
> Savoie Technolac, BP 352
> 73377 Le Bourget du Lac Cedex
> http://www.camptocamp.com
>
>
> _______________________________________________
> Mailing list: https://launchpad.net/~openerp-community
> Post to     : openerp-community@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~openerp-community
> More help   : https://help.launchpad.net/ListHelp
>



-- 
Niels Huylebroeck
Lead Architect   --   Agaplan
Tel. : +32 (0) 93 95 98 90
Web : http://www.agaplan.eu

Follow ups

References