Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[15.0][UPD] apriori + analysis + coverage #3460

Merged
merged 7 commits into from
Jul 21, 2022

Conversation

MiquelRForgeFlow
Copy link
Contributor

Here it goes.

Copy link
Contributor

@legalsylvain legalsylvain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hum. thanks for this work.

  1. do you know why there are such changes between few monthes ?
  2. Some changes are annoying. I don't know how to handle the account case (for exemple). Do you have an idea ?

thanks.

regards.

@MiquelRForgeFlow
Copy link
Contributor Author

  1. Hehe. Do you now how many commits odoo have added in those months? First year after new versions is always like this.
  2. Don't worry. For account, some scripts will be added in 15.0.1.1 version and maybe some others in 15.0.1.2 folder. Depending of what has changed between them. To know what has changed between them, we will need to manually compare both analysis and detect the difference.

@legalsylvain
Copy link
Contributor

Don't worry.

Hum, I'm not sure : I'm mean,

  • the current upgrade.txt file in the folder 15.0.1.1 is a diff of 14.0 and 15.0.1.1
  • the current upgrade.txt file in the folder 15.0.1.2 is a diff of 14.0 and 15.0.1.2

AFAIU, the current upgrade.txt file in the folder 15.0.1.2 should be a diff of 15.0.1.1 and 15.0.1.2 (maybe minor).

What do we do ?

Option A) We assume two revision.15.0.1.1 and 15.0.1.2 and we provides script for both "migrations". We have to change the upgrade.txt file 15.0.1.2 to be only the diff between 15.0.1.1 and 15.0.1.2. And we should take care that in the migration, the execution will be the following :

  • 15.0.1.1/pre-migration.py
  • 15.0.1.2/pre-migration.py
  • 15.0.1.1/post-migration.py
  • 15.0.1.2/post-migration.py

So, in the pre-migration of the 15.0.1.2 the database is not totally a "15.0.1.1" (15.0.1.1/post-migration.py has not been executed). I faced similar issue when upgrading queue_job. (see : https://github.com/OCA/queue/tree/12.0/queue_job/migrations)

Option B) : we remove all the 15.0.1.1 folder. but it requires to rebase / adapt all the current (#3284) for instance.

CC : @StefanRijnhart, @pedrobaeza : what do you think ?

@pedrobaeza
Copy link
Member

Everything should be done directly in 15.0.1.2 and directly the other folder must disappear.

@jcdrubay
Copy link
Contributor

Then does it make sense to keep the name 15.0.1.2, why not only keeping 15.0? or why not simply removing that folder from the path?

@pedrobaeza
Copy link
Member

The folder name must always match the latest version

@legalsylvain
Copy link
Contributor

Everything should be done directly in 15.0.1.2 and directly the other folder must disappear.

Well, I'm in favour of this solution (for simplicity). But I'm not very confortable with the fact to say to 15.0 contributors, to add extra work to rebase and recheck diff, and fix some stuff. The work provided here #3287 is important, and it lacks code review. (we should talk about that topic in the next coming soon Openupgrade Strategy meeting).

Then does it make sense to keep the name 15.0.1.2, why not only keeping 15.0? or why not simply removing that folder from the path?

It doesn't work. And it doesn't solve the main problem. (1 folder or two folders)

The folder name must always match the latest version

well, it is possible to have multiple folders, but it's a mess.

@MiquelRForgeFlow
Copy link
Contributor Author

I adapted the account issue:
Selection_491
Selection_492

@pedrobaeza
Copy link
Member

Well, I'm in favour of this solution (for simplicity). But I'm not very confortable with the fact to say to 15.0 contributors, to add extra work to rebase and recheck diff, and fix some stuff. The work provided here #3287 is important, and it lacks code review. (we should talk about that topic in the next coming soon Openupgrade Strategy meeting).

Complain to Odoo, not to us. The work is the same even if the PR is merged: the scripts must be passed to the other folder and recheck the diff. If not, migration may not work.

@MiquelRForgeFlow MiquelRForgeFlow force-pushed the 15.0-imp-analysis branch 5 times, most recently from fea6394 to e0c5e77 Compare July 20, 2022 13:17
@MiquelRForgeFlow
Copy link
Contributor Author

Ready

@MiquelRForgeFlow
Copy link
Contributor Author

I tested the mail_group scripts in local, I fixed some things, and now it works like a charm :)

@pedrobaeza pedrobaeza merged commit d1a40ce into OCA:15.0 Jul 21, 2022
@pedrobaeza pedrobaeza deleted the 15.0-imp-analysis branch July 21, 2022 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants