Archives
Contributors
Re: Procedure to create 16.0 branches
>> 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.
I don't remember the details right now, as the other procedure has been absent for a long time, but using such instructions generates conflicts on several occasions. Trust me, I suffered from them in my OCA beginnings...
>> 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.
Why will it fade away? A new module can be introduced in any version, so this will always happen.
>> 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.
I have put an example, but there are several other things that will conflict, like packages, environment variables, etc.
And you are missing very important points, like the one about the trust of a new migration PR if it contains missing commits or not. When a contributor adds a pull request and it only contains the migration commit, is it because there's no missing commits in the prior branch, or is it because he/she has missed them as the natural tendency for a newcomer is to just change installable to True and test if it works, and then commit the changes? This transfers a lot of responsibility to the reviewers to check this. With the current approach, it's very easy to detect an incorrect migration PR.
Note that the problem about missing commits is very common, as the migration of the modules happens a lot of time after the new version is released, as people need to incorporate the new version in their flows.
But the main important thing is that I don't see any clear exposed advantages to change the approach. The ones you have mentiones:
> 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.
The same security problem with missing commits.
> 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.
The same CLA bot issues with missing commits.
> 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.
Such things have already been developed (thanks Sylvain!), so this is not a problem anymore.
> Slow git repo growth: by avoiding the recreation of identical commits in several branches we would slow down the git repo size increase.
As said, this is not true depending on your git usage (single branch clone), or with the deprecated modules (double diff, adding and removing stuff).
Regards.
by Pedro M. Baeza - 05:56 - 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.
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 - 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.