[UPD] Analysis

[StefanRijnhart]

No description provided.

[NL66278]

👍 LGTM

[pedrobaeza]

I'm wondering why the field picking_warn (

OpenUpgrade/addons/stock/models/res_partner.py

Line 17 in bb64e03

picking_warn = fields.Selection(WARNING_MESSAGE, 'Stock Picking', help=WARNING_HELP, default='no-message', required=True)

) is not being detected in the analysis file.

@StefanRijnhart do you have any clue?

[hbrunn]

I'd like to merge this rather sooner than later, any objections @pedrobaeza ?
@StefanRijnhart if you touch the analysis module anyways, do you see a simple way to add a flag to deleted xmlids that are noupdate? Usually, we'll have to do something with them, so such a flag would be very helpful.

[hbrunn]

@StefanRijnhart picking_warn used to be defined by the warning module which is gone now, any possibility this confuses the analysis?

[StefanRijnhart]

Ah yes, of course. Because this kind of warning is logged in the originating module which is now missing, the analysis line is syphoned off to the general log: https://github.com/StefanRijnhart/OpenUpgrade/blob/eefdb93123b3aea6ae5ede6d5dd19668ccd36875/odoo/addons/base/migrations/10.0.1.3/openupgrade_general_log.txt#L207

[pedrobaeza]

Then why picking_warn is not reported on the module analysis file?

Should we merge this?

[StefanRijnhart]

@pedrobaeza I tried to say that it is reported, in the general log. So yes, please merge this.

[pedrobaeza]

But reported on the general log doesn't serve, as people will expect to see that change on the analysis file of the module itself. Can't we add that text on both module analysis files (the source and the target)?

[StefanRijnhart]

@pedrobaeza yes, that would be a good idea. But it is totally irrelevant to the merge status of this branch.

[pedrobaeza]

No, it isn't, as if this feature is integrated, analysis files will change again.

[StefanRijnhart]

Totally irrelevant again. Having a separate PR for an updated analysis due to an upstream code merge and one for a modified analysis process is strongly preferred

[StefanRijnhart]

besides, I am not going to work on this new feature of the analysis now.

[pedrobaeza]

Well, let's merge it, but I'd prefer to have it because then migration scripts are not going to be correct without knowing that. Please consider to add the feature, as you are the one that knows better the module.

[StefanRijnhart]

@pedrobaeza yes you are right. Here you go: #917

[pedrobaeza]

Thank you very much, Stefan!

[MiquelRForgeFlow]

@StefanRijnhart @pedrobaeza

Now that #895 is merged, shouldn't a new analysis like this PR be done? 🤔

For example, in this PR is added the line https://github.com/OCA/OpenUpgrade/pull/871/files#diff-9559914596ccb1f4397871ea751e6b2aR2, and thus a new Todo. But with the merged upstream of v9, that field is fixed (becomes required) so then that Todo is not a Todo anymore. Well, I don't now if I explained it correctly.

[pedrobaeza]

It shouldn't happen as Odoo stable policy disallows to change DB layout. 10.0 was an exception, as the DB layout wasn't freeze yet when the analysis was done.

[hbrunn]

I'm running my first real world migration with this, and think we need to handle customized views some way or the other in this module's migration. 9 is the first version with the editor that creates noupdate=1 changes on ir_ui_view records. Currently, I just reset the noupdate flag in my premigration (otherwise customized views not matching current views break everything), but that's not going to get us happy customers. Do you have ideas how to handle that better?

My only other idea would be to also backup the modified architecture, and try per view if we can write the modified architecture after the upgrade. If it raises, we move the modified code somewhere for later manual update, if it doesn't, we're happy (and users who actually want the new version can simply use the reset functionality). This could be quite complex and time consuming, we also need to do the writes in reverse order of hierarchy I think

[pedrobaeza]

I think you refer to website views (which is a bit unrelated to this PR), but let me tell you about my experience in that: indeed, is a pain to migrate this kind of DBs, as you have to manually check for each view. Till now, this has been my way of working, updating module and looking for errors, patching them through SQL, and iterate again.

An automatic checking that discards wrong views and store them in any place for later review. You have to also deal with views created from scratch that fails, so there's no alternative arch to load, so we must supply a neutral arch in that cases.