Contributors mailing list archives
contributors@odoo-community.org
Browse archives
Re: Reviews
by
Ecosoft Co. Ltd., Kitti Upariphutthiphong
Hi Tom,
So, frankly, I don't mind having a lot of new PRs, even unmerged ones. When I got stuck, they also useful.
Thanks for bringing this up. I experience the same. May I share opinion as a not so advanced developer side.
If I understand correctly, the policy of OCA is to welcome more contributions with maturity levels like alpha. If this is the policy, we will have a lot of PR (with many low quality ones too). I think good / interesting PR are still merged fast, and natural selection will kill the low quality module in the long term anyway.
So, frankly, I don't mind having a lot of new PRs, even unmerged ones. When I got stuck, they also useful.
But I like to support some idea mentioned here, which could help balance that.
- Easier to mege, but with some quality constraints. I.e., [ADD] modules with full test script, can merged with 1 reviewer.
- Close the inactivity PR very fast, less poison to reviewers
- The [MIG] module should be easier to merge, i.e., if a full test is added no reviewer is required, or maintainer can do the merge himself.
I think we should use Test Coverage as an incentive to contributors if he/she want to get merge faster.
Kitti U.
PS: Thank you Daniel, for sharing the article. It is interesting and may fit with nature of Odoo nowadays.
On Thu, Sep 16, 2021 at 7:51 PM Sylvain LE GAL <sylvain.legal@grap.coop> wrote:
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.17Service Informatique : (+33) 09.73.79.64.40Astreinte Informatique : (+33) 06.81.85.61.43Member 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 processRecently, 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 ithum. 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
_______________________________________________
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 -
-
-
