[12.0] base: Revert "[IMP] OpenUpgrade: don't delete xmlids linked to nothing" #2236
Conversation
This reverts commit 93d0f7e.
|
But this is not the same as we are debating in the other place. This is for XML-IDs, not for records themselves. |
|
We debated this in #2011. In that PR, I initially proposed apply the change that now I am trying to revert. In that PR, Stefan said that there was no point to preserve dangling XML ids (entries in ir_model_data that refer to a res_id that is no longer in the model's table, or simply, xmlids linked to nothing) so I didn't apply it finally. Thus, that's why I am reverting it here. |
|
Check #2223 (comment) where the pointed commit is not this one (but one with same name): 0ddea82 |
|
I am not speaking of #2223 (comment) nor 0ddea82. Do not relate one thing with the other or we will never get out of this stupid loop. What I am saying is that in #2011 I initially proposed the change I am trying to revert now in this PR. Stefan pointed that there was no sense to save xmlids linked to nothing and thus pointed to the correct solution that I finally implemented in that branch (0ddea82) that now in #2223 we have reverted. |
|
OK, I have a little mess with all of that: /ocabot merge |
|
What a great day to merge this nice PR. Let's do it! |
|
@pedrobaeza your merge command was aborted due to failed check(s), which you can inspect on this commit of 12.0-ocabot-merge-pr-2236-by-pedrobaeza-bump-no. After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red. |
|
/ocabot merge |
|
What a great day to merge this nice PR. Let's do it! |
|
Ok, I think I do agree with removing this because indeed it does not make any sense to preserve XMLID's of which the record is already removed, like you say. Note that cdc470e#diff-bb1e3d0f2e95b26bcd024950824e022fR1955 preserves noupdate records and their XML ids. I think that should cover the important cases. |
|
Uhm, isn't that what this PR is removing? I confess I'm pretty lost right now |
|
Odoo by default removes XML ids for which the record no longer exists. Miquel introduced a change earlier on so that those dangling XML ids were preserved. This PR reverts that change. |
|
Then should we continue with this PR or not according your opinion? |
|
Congratulations, your PR was merged at f195c1c. Thanks a lot for contributing to OCA. ❤️ |
|
As you guessed, yes please continue ;-) |
4 participants
This reverts commit 93d0f7e.
It was long ago, I didn't remember. And that was wrong.
--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr