Contributors mailing list archives

Browse archives


Re: Reviews

ClosingAp Open Source Integrators Europe, LDA, Daniel Reis
- 16/09/2021 14:02:31

Thanks for opening the discussion.

The board worked on a procedure introducing module maturity levels, and it involved lowering the barriers to merging.
Maintainer roles and ocabot ale tried to improve this.

Having active maintainers is really important for things to work, so I personally appreciate your suggestions.

Food for though, I recently found this an interesting read, and I wonder how close we could get of it.


On 16/09/21 12:51, Tom Blauwendraat wrote:
Hi all,

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

Also, even if there are 2 reviews then PSC's generally don't respond to 
merge requests.

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

- 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.

- 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.


Post to:


  • Avatar


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

    10 replies 10 replies