Skip to Content
  2. Mailing Lists
  3. Contributors
  4. Re: Reviews

Archives

Contributors

Re: Reviews

Re: Reviews

Re: Reviews

El vie, 17 de sep de 2021 a las 07:41:54 AM, Tom <tom@sunflowerweb.nl> escribió:
Regarding the "smarter bot" ideas: although it's typical for a bunch of programmers to come up with a solution to a problem by throwing more engineering into it, I feel there's something to it, it just needs more crystallization and then a hackathon to make it happen. 

Let's take for example Firefox.

Firefox has the desktop product, and it builds in different platforms: Linux, Windows, Mac... How to handle that? They have a build system that applies conditionals ("if", "case" and similar) to produce the platform-compatible artifact. Usually this is a Makefile, but there are other systems for the same. Result: single code base, multiple platforms.

However it also has Android and iOS versions, which are almost completely different. They share some essential concepts, but code bases are split. This is more complex to handle, but they end up supporting these extra platforms too. Result: split code base, single platform.

As you can imagine, the desktop product is more maintainable.

Now, OCA. Our products are, mainly:
  1. Addons.
  2. OpenUpgrade.
  3. OCA tooling.
We only follow the "single code base, multiple platforms" approach in OCA tooling.

For OpenUpgrade, we use "split code base, single platform", and it would be impossible to change that.

And for addons? The platforms they run on are different versions of Odoo. I always believed it's possible to have a single code base, but that's a dead discussion and simply won't happen, so the best we can do is improve what we have. And what we have is "split code bases, single platform". And IMHO the way to improve it is a fwbot, just like Odoo does.

Maybe a good theme for a team at this year's online code sprint?

Indeed.
by Jairo Llopis - 12:01 - 17 Sep 2021

Reference

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

Tom
    by Tom Blauwendraat - 01:51 - 16 Sep 2021