Archives
Contributors
Re: Procedure to create 16.0 branches
Hi,
From my point of view, a change that would need more PSC dedication to make it work is not a good idea. With this proposal, that's what it looks like to me, PSCs would need to do these tasks that are not needed now:
- Review obsolete/discontinued modules modules and create a PR to remove them.
- Pay attention to missing commits on migration PRs. It is true that this can happen now if a migration PR is open for a long period (while the PR is open new changes can be merged in previous versions), but the chances are lower and almost zero at the moment of opening the PR (zero if the contributor fetches just before starting to migrate).
- Attend more CI issues on forward ports as they will be more frequent. I would say that currently a bot that forward ports stuff is a "nice to have" but with this proposal it would be a "must have".
On top of this, in point one I think it is not an easy task either. Think about the following scenario: module A is in branch 16.0 of repo R but not migrated yet, 1 year goes by and therefore the PSC proceeds to remove module A. Then a couple of months later someone finally migrates module A to version 16, he/she will re-add all commits and code, so 3 big diffs for a single moduleone mor. This highlights a question that has a difficult answer for me: How long does it take for a module to be obsolete and be removed?
From the perspective of a contributor/developer, there is also a problem that annoyed me a lot when branches were created with installable false, and it was that you will find stuff that is not relevant when searching things in your IDE as the non-migrated code is there. I think it is worth thinking about this as it will affect the day-to-day work of frequent contributors.
Also, when looking at repositories browsing github it is now very quick and easy to see what is dead, what is not migrated to the latest version, etc. you just need to see if there is something in a given branch in a given repository. It is true that the README of the repo can help, but be honest... I never scroll down to see the README, I don't think many people would do that intuitively.
I think the proposed approach can work for a number of repos, but not for the majority:
- imagine sale-workflow without the natural cleaning of the clean start each year.
- imagine a repository that gets contributions in v15 and then never again in next versions, the modules will be copied during years to newer versions.
Obviously these things can be mitigated but I think now modules and repositories "die" more naturally.
Therefore, I like the idea proposed by Holger, I think the branch initialization mode can be have 2 options being the current mode the default, the new mode for sure can work for specific repos with very defined scope and clear and active PSC like mis-builder (I could even give it a try in ddmrp).
My 2 cents :)
Kind regards,
On Wed, Jul 20, 2022 at 8: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
Moreover, for your size problem, current behaviour is taking more place than proposed one as new main branches are sharing same commits with preceding one.You can test it locally (I did it) with git rev-list --disk-usage --objects --allSo, that point is solved.On Wed, Jul 20, 2022 at 7:57 PM Pedro M. Baeza (Tecnativa) <notifications@odoo-community.org> wrote:Dennis, the images unfortunately are not shown (Odoo/OCA instance bug), but the point in `This will increase the size of the repository the same, as no common ancestor and then different SHA.` is that it will affect the same on size on one procedure or the other.> I see advantages here as currently if a module is not there, that does not mean if it is migrated yet or has been deprecated. With proposed approach, you can detect it more easily (present in previous branch, not in current. Moreover in the commit history you can see why it has been deprecated and learn about changes you maybe missed).I don't get your point here, but it's the same good or bad, as it depends when a possible new module has arrived on prior branches.> From several same approach repositories knowledge, I would say this will lead to a cleaner situation and an easier newcomers contribution processes understanding.I have already expressed why it won't be cleaner and the problem not only for newcomers, but for existing contributors or reviewers.Regards._______________________________________________
Mailing-List: https://odoo-community.org/groups/contributors-15
Post to: mailto:contributors@odoo-community.org
Unsubscribe: https://odoo-community.org/groups?unsubscribe
--_______________________________________________
Mailing-List: https://odoo-community.org/groups/contributors-15
Post to: mailto:contributors@odoo-community.org
Unsubscribe: https://odoo-community.org/groups?unsubscribe
Lois Rilo Antelo
Odoo consultant at ForgeFlow S.L.
by Lois Rilo Antelo - 08:46 - 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.