Skip to Content
  2. Mailing Lists
  3. Contributors
  4. Re: Procedure to create 16.0 branches

Archives

Contributors

Re: Procedure to create 16.0 branches

Re: Procedure to create 16.0 branches

Re: Procedure to create 16.0 branches

Pedro,

Let me answer what I perceive has the most important points:

> Also derived from the first point, it's the lack of homogeneity in the processes. With the current one, everything follows the same procedure. This is VERY IMPORTANT: to have only one way of doing everything, not having to know different processes depending on the state of the branch.

Please note that the current procedure with "git format-patch | git am" will continue to work as before: it will just pick up missing commits. There is no obligation to learn new tools.

> Modules being migrated with a jump of version will have the same problem of not having any of the commit history.

That is true but this will fade away over time.

> Rebel modules configuration and similar CI tricks may cause problems on new branches.

All the CI is now configured with copier questions, and we will reset the rebel module groups when creating new branches. So that should not be a problem.

-sbi


On Wed, Jul 20, 2022 at 4:52 PM Pedro M. Baeza (Tecnativa) <notifications@odoo-community.org> wrote:
Returning back to this approach is a mistake IMO, as it has more cons than pros. First of all, one of the advantages that you mention is not correct, or at least, depending from the perspective, that is the "slow git repo growth": if you download a single new branch, it will cost a lot more, as it has all the commit history from all the modules. This gets even worse when the modules are deprecated (more on this later).

And about the drawbacks, there are still bad interactions with modules with previous versions code due to Python or ORM peculiarities (check for example this PR for fixing a code that shouldn't be executed not being the module installed), and for sure more will appear. Going to the process of discovering these problems and fixing this is an unneeded maintenance burden. The discoverability will have the same problem, as people don't get used to having useless folders an d to need to go to that file to check it or open the manifest. Even more, I think the OCA apps store module will have problems with this approach (but that's only suspicions).

Now let me summarize again the rest of the cons that were the reason to change to this approach:
  • When the branches are created for the new 16.0 version, activity in the prior branch 15.0 may happen until the migration of the module is done, but the 16.0 branch won't reflect these new commits. There's a high risk of the contributors of the migration not including latest changes, or to work on the deprecated source code and come to OCA with their pull request to be discouraged by one qualified reviewer saying that they need to rework the migration including the missing commits. Forcing and teaching people to use tools like oca-port is a high entry barrier. Someone has proposed to develop a bot for automatically doing these forward-ports, but it will have lots of problems as well:
    • First of all, it needs to be developed, which is not an easy task and needs to be done. Adding extra tools for something that is not needed with another approach is not the convenient way to go.
    • If there's already a migration PR proposed, the new commits won't be there, and it will require a rebase by the migrator (again more contribution friction).
    • Those extra commits will lose the security signing that may be present in previous branches. The only way to not lose them is that the same author makes the commit signing it in the new branch.
    • There can potentially be the same CLA problems.
    • When the bot does the forward-ports, CIs may be red due to external reasons, but then these commits won't never reach the new branch unless manual actions are done.
    • This will increase the size of the repository the same, as no common ancestor and then different SHA.
  • Derived from this, it requires that the reviewers alw ays need to check if the migration pull request contains the latest commits. With the previous approach, there are less chances that they are not included, and we light up the reviewing process.
  • Also derived from the first point, it's the lack of homogeneity in the processes. With the current one, everything follows the same procedure. This is VERY IMPORTANT: to have only one way of doing everything, not having to know different processes depending on the state of the branch.
  • It will generate extra burden for deprecated modules, requiring to make a pull request to remove them, and creating a very big diff commit of such removal, increasing the branch and repository size (remember that there will be at least one commit adding the module, and another removing it, so double diff).
  • Modules being migrated with a jump of version will have the same problem of not having any of the commit history.
  • Rebel modules configuration and similar CI tricks may cause problems on new branches.
  • In the past, having uninstallable modules has cost a lot of problems. Remember the static folder problem that loads the Python files, the switch from version 2 to 3. Now they are not present, but nothing prevents a new side effect from happening in the future. Better to have the house cleaned.
The fact that some of you are using this approach in some repositories is not representative, as that repositories have a very limited scope (few modules), and very controlled by few contributors, but try it in OCA/sale-workflow or OCA/purchase-workflow...

Please reconsider your approach. If not, we at Tecnativa don't know if we can afford this process in our contributions to OCA.

Regards.

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

by Stéphane Bidoul - 05:30 - 20 Jul 2022

Reference

  • Procedure to create 16.0 branches
    Dear contributors,

    I'm starting to think about the process to create the 16.0 branches. And the more I think about it, the more I'm convinced we should do it by adding "installable": False in the module manifests, instead of creating empty branches.

    This would have several benefits:
    • Improve security. Indeed, currently migration PRs have a lot of commits and reviewers only look at the last 2 commits. By accident or malice, it would be easy for a contributor to sneak bad code in older commits, that would go unnoticed. As the community grows, I think this a very important topic.
    • Avoid CLA bot issues: currently, the CLA bot is flagging old commits that were ok at the time they were created, but may not be valid today as contributors may have changed email, or revoked their CLA.
    • Reduce oca-github-bot complexity: work has to be done to make the bot aware of other branches in migration PRs (notably to look-up maintainers). This would be unnecessary if a migration PR is a normal PR to an existing addon directory. On the contrary, the bot could even detect migration PRs automatically by noticing the change to the installable flag, so this could simplify some processes.
    • Slow git repo growth: by avoiding the recreation of identical commits in several branches we would slow down the git repo size increase.
    About the possible drawbacks, I am under the impression that all the reasons we had back then to create empty branches have faded away:
    • Today, Odoo and all the OCA tooling work perfectly well when there are addons marked as uninstallable. They are correctly ignored by linters, tests, and Odoo does not attempt to import the code.
    • Regarding discoverability, the addons table in the README shows a clear view of what is not migrated.
    The migration procedure and tools should continue to work as today, to pick up commits that would have been added after branching (basically the git-am process would simply work as it does today)

    All we'd need maybe is to agree on a process to remove modules that have not been migrated for several versions. But in a first approach, regular PRs to remove now useless modules would probably be sufficient.

    Are there any other arguments (pro or con) that I would have missed ?

    Looking forward to reading your feedback on this proposal.

    -Stéphane

    by Stéphane Bidoul - 12:45 - 20 Jul 2022