Archives
Contributors
Re: Procedure to create 16.0 branches
Hi all,
I like better the current method (having only working/tested/maintained modules in each branch) for the following reasons :
- I find it easier to see what is available un a given version, what would still need to be migrated, and also to get an idea of which versions are being actively used (though I guess I could look at READMEs, migration issues and overall statistics to get the same info)
- on each Odoo instance we deploy we download specific branch and having lots of unusable code would increase bandwidth and storage (although mostly text that could be compressed, so probably not dramatic neither)
I agree that the process to migrate a module from one version to another is not really easy (at least the first times), but I understand that it would still probably need to be done in order to make sure you get the latest commits from previous branch. I am afraid that this would often be forgotten if not strictly necessary in migration process, and for sure we do not want to transfer this extra burden of checking for each PR whether based on latest commits or not to reviewers / PSC.
Last, I am not in favour of having 2 different processes that can be applied per repo, since it would increase complexity for understanding why 2 repos are not behaving the same, and we may lose newcomers (but also for people not so much involved in OCA) - although here again I understand from discussions this is already the case for some repos.
My 2 cents...
Best Regards,
Rémi CAZENAVE
SCOP Le Filament
Hello,My vote goes to the opt-in option at least unless Pedro get convinced, because going against the will of somebody processing such a large portion of the PRs would be a terrible shot in the feet.That being said, earlier Pedro raised the concern about the possibility to craft an attack commit inside a missing commit that would be cherry-picked just like with the current way of migrating. But:On Wed, Jul 20, 2022, 3:42 PM Roussel, Denis <notifications@odoo-community.org> wrote:To summarize:
- commits SHA are different with current behaviour
- commits SHA are equal with proposed one
Denis could you confirm this SHA conservation could make us safe against such crafted attack commits in the middle of the missing commits that one would need to cherry-pick with the new procedure?This crafted commit attack thing is indeed extremely concerning..._______________________________________________
Mailing-List: https://odoo-community.org/groups/contributors-15
Post to: mailto:contributors@odoo-community.org
Unsubscribe: https://odoo-community.org/groups?unsubscribe
by Rémi Cazenave - 10:01 - 21 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.