Contributors mailing list archives
contributors@odoo-community.org
Browse archives
Re: Reviews
by
Groupement Régional Alimentaire de Proximité, Sylvain LE GAL
Hi,
Thanks for the topic.
Some response in your email.
kind regards.
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
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
-
Reviews
bySunflower IT, Tom Blauwendraat- 16/09/2021 13:49:25 - 0-
-
-
-
-
-
-
-
Re: Reviews
byGroupement Régional Alimentaire de Proximité, Sylvain LE GAL- 16/09/2021 14:50:21 - 0 -
-
-
