Contributors mailing list archives

contributors@odoo-community.org

Browse archives

Avatar

Re: Reviews

by
Groupement Régional Alimentaire de Proximité, Sylvain LE GAL
- 16/09/2021 14:50:21
Hi,

Thanks for the topic.

Some response in your email.

kind regards.

Sylvain LE GAL - Twitter
GRAP - Service informatique (Groupement Régional Alimentaire de Proximité)
Site Web | FramaSphere | Facebook
3 Grande rue des Feuillants, 69001 Lyon
Standard : (+33) 09.72.32.33.17
Service Informatique : (+33) 09.73.79.64.40
Astreinte Informatique : (+33) 06.81.85.61.43
Member of the OCA (Odoo Community Association)


Le jeu. 16 sept. 2021 à 13:51, Tom Blauwendraat <tom@sunflowerweb.nl> a écrit :
Hi all,

For years, OCA has a big problem with unmerged PR's.

That's true ! One of the origins is the fast release rate of Odoo, which requires continuous migration of modules that, for many, do not change.

Unfortunately, that situation will not change. Odoo policy seems to be : "a year = a release".

let's put it into perspective a little bit :

- maintainers is a quite "new concept" in the OCA. We should continue to promote the concept.
- PSC members should be updated. I worked recently to update PSC member for OCA/pos, here removing for exemple a guy that stopped working on odoo 5 years ago... I think it should be generalized.
- we generally organize code sprint to merge PRs, but with the covid, it's been two years since there was a "real" event (only online), and I think this has disturbed the process

Recently, we have added on the repo homepage, the maintainer icons. This allows to see quickly which modules are not maintained. The result is good.

Other recent improvment has been done in the bot and maintainer-tools project. I think that allowing maintainers to merge "[MIG]" PRs could be a great addition.
Also, even if there are 2 reviews then PSC's generally don't respond to 
merge requests.
That depends a lot of PSC and repo. some PSC of some repo are very reactive.
The answer has always been "let's review more" or "Let's use 
gitaggregator and so we can use unmerged PR's".

But why don't we try something more radical:

- Let's write a script to assign "maintainer" role for all modules to 
the person who committed the oldest/original version of it
hum. That can be a good idea, if the automation is not total. I mean, if we create a PR, and tag people saying : "The bot want to set you as maintainers for module x, y, are you agree ? please answer" --> it's ok.
If we affect automatically people, I think we'll have modules with maintainers that don't maintain modules for real. it's worse than not having maintainers.
Some modules are 10 years old, their authors may not work on odoo at all.
- If maintainer does not respond to a ping longer than 1 month he loses 
the maintainer role, which then changes to the default maintainer that 
is set for the full repo.
Oh, no ! I'm PSC of OCA/pos. I don't want to have my name set to all modules that are unmaintained. We should assume that there are unmaintained modules. That's the reality.
- Let's require only 1 positive review from now on. After that the 
maintainer can merge.

--> I don't think that having 2 reviewers is always necessary and it 
also does not prevent bugs from being merged - this happens anyway. I 
used to have high trust of merged OCA modules but after seeing some 
quite ugly bugs and incomplete work being merged I am starting to think 
that maybe the quality of the unmerged stuff is not that bad as compared 
to what is actually merged. The maintainer can prevent really bad 
changes from entering, by just closing the PR.
As said by daniel, that is handled by the current rules, depending on the maturity.
Tom


_______________________________________________
Mailing-List: https://odoo-community.org/groups/contributors-15
Post to: mailto:contributors@odoo-community.org
Unsubscribe: https://odoo-community.org/groups?unsubscribe

Reference

  • Avatar

    Reviews

    by
    Sunflower IT, Tom Blauwendraat
    - 16/09/2021 13:49:25 - 0

    10 replies 10 replies