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

[16.0][OU-FIX] Base: Add help in ir.actions.act_window translation exclusions #3866

Merged
merged 1 commit into from
May 11, 2023

Conversation

cyrilmanuel
Copy link
Contributor

@cyrilmanuel cyrilmanuel commented May 11, 2023

Add fields help in exclusion of ir_act_window

The pre-migration script exclude some fields from models, inherited by ir.actions fields. the objectif is allow the conversion of all text fields in json or jsonb (mecanic in odoo 16). but if the field is inherited by another fields in another model ( in my exemple name and help in the model ir_act_window are inherited by ir.actions fields) you cannot execute the sql request. the sql request will failed because you cannot convert fields inherited.

@cyrilmanuel cyrilmanuel changed the title Fix base migration field help in exclusion [16.0][FIX] Base pre-migration script, add field help in exclusion May 11, 2023
@pedrobaeza pedrobaeza added this to the 16.0 milestone May 11, 2023
@pedrobaeza
Copy link
Member

I don't get what do you mean by "inherited". Also the commit message doesn't have the proper format: https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#commit-message. For OU related stuff, we put [OU-TTT] tag, being TTT the operation done.

@cyrilmanuel
Copy link
Contributor Author

hello @pedrobaeza, the pre-migration script exclude some fields from models, inherited by ir.actions fields. the objectif is allow the conversion of all text fields in json or jsonb (mecanic in odoo 16). but if the field is inherited by another fields in another model ( in my exemple name and help in the model ir_act_window are inherited by ir.actions fields) you cannot execute the sql request. the sql request will failed because you cannot convert fields inherited.

i hope it's better to you to understand.

@legalsylvain
Copy link
Contributor

Hi @pedrobaeza. Thanks for your comment.

For OU related stuff, we put [OU-TTT] tag, being TTT the operation done.

Documentation question (I have to write the "how to contribute"):

What is the reason for that exception ? I never asked and I guessed that it was usefull in the previous design, when OpenUpgrade was a fork and when openupgrade commits was mixed with odoo commits to make better identification of "openupgrade" commits. Is this the case ? Are there any other reasons ?
If no, we can stop to apply this rule from the v14, as now, all the commits are openupgrade commits. don't you think ?

Kind regards.

@pedrobaeza
Copy link
Member

Yes, thanks. Please rephrase the commit message with the proper format and include this explanation in the expanded commit message.

@pedrobaeza
Copy link
Member

Documentation question (I have to write the "how to contribute"):

What is the reason for that exception ? I never asked and I guessed that it was usefull in the previous design, when OpenUpgrade was a fork and when openupgrade commits was mixed with odoo commits to make better identification of "openupgrade" commits. Is this the case ? Are there any other reasons ? If no, we can stop to apply this rule from the v14, as now, all the commits are openupgrade commits. don't you think ?

It's still needed, as branches from 14.0 are still a mix of a module (openupgrade_framework) with migration scripts (that are paradoxically inside another module, openupgrade_scripts). If you fix the framework, you put [FIX] openupgrade_framework: ..... But if you add, improve or fix a migration script, technically you are touching openupgrade_scripts module. That's why [OU-ADD] <odoo_core_module_for_which_you_are_adding_the_script>: ... comes to play, for being correct on what we are doing without breaking the current module-related format meaning.

@cyrilmanuel cyrilmanuel changed the title [16.0][FIX] Base pre-migration script, add field help in exclusion [16.0][OU-FIX] Base pre-migration script, add field help in exclusion of ir_act_window May 11, 2023
@cyrilmanuel
Copy link
Contributor Author

Thank's @legalsylvain and @pedrobaeza for your precision !

let me know if i'ts all done for you

@pedrobaeza
Copy link
Member

The commit message should be:

[OU-FIX] base: Add `help` in ir.actions.act_window translation exclusions

The pre-migration script excludes some fields from models, inherited by ir.actions fields.
The goal is to allow the conversion of all text fields in json or jsonb (mecanic in odoo 16),
but if the field is inherited by another fields in another model (in my example, name and help
in the model ir_act_window are inherited by ir.actions fields), you cannot execute the sql request.
It will failed because you cannot convert fields inherited.
  • Without Odoo version number
  • Proper module technical name.
  • Better main message.
  • Including the extended reason.

@legalsylvain
Copy link
Contributor

It's still needed, as branches from 14.0 are still a mix of a module (openupgrade_framework) with migration scripts (that are paradoxically inside another module, openupgrade_scripts). If you fix the framework, you put [FIX] openupgrade_framework: ..... But if you add, improve or fix a migration script, technically you are touching openupgrade_scripts module. That's why [OU-ADD] <odoo_core_module_for_which_you_are_adding_the_script>: ... comes to play, for being correct on what we are doing without breaking the current module-related format meaning.

Thanks for your explanation. I was thinking of something lighter and maybe more common for newcomers like :

- [ADD] account: initial migration script ...
- [IMP] stock: performance ...
- [FIX] base: add help in translation
- [MIG] openupgrade_framework. (migration from 15.0 to 16).
- [FIX] openupgrade_framework: ...

That being said, it's just a suggestion, and as the main contributor and merger of OpenUpgrade, you choose !

have a nice day.

@pedrobaeza
Copy link
Member

But with such nomenclature, you don't really know by tag what you are doing. You have to check the module and deduce that the indicated module is one of the core modules (or OCA ones, as we perform also merge operations and so), and then the commit is for something related to migration scripts. Prefixing with OU, there's no such doubt.

Right now, there's only one extra module (openupgrade_framework), but the situation may change on the future.

And finally, we are homogeneous across versions (although 13.0 and below is getting now very old, but doing the same on all versions is always good), and there's still activity on such branches: #3848

@legalsylvain
Copy link
Contributor

@pedrobaeza : I'm totally fine with it! I'd do a PR on the documentation, which includes what you say.
@cyrilmanuel : sorry for occupying your PR with off-topic questions ;-)

…ions

The pre-migration script excludes some fields from models, inherited by ir.actions fields.
The goal is to allow the conversion of all text fields in json or jsonb (mecanic in odoo 16),
but if the field is inherited by another fields in another model (in my example, name and help
in the model ir_act_window are inherited by ir.actions fields), you cannot execute the sql request.
It will failed because you cannot convert fields inherited.
@cyrilmanuel
Copy link
Contributor Author

@legalsylvain : ho no, thanks for your question, it's help me to understand what is the diff between OU-FIX and FIX. thanks @pedrobaeza for your help and all explanation !

@cyrilmanuel cyrilmanuel changed the title [16.0][OU-FIX] Base pre-migration script, add field help in exclusion of ir_act_window [OU-FIX] Base: Add help in ir.actions.act_window translation exclusions May 11, 2023
@pedrobaeza pedrobaeza merged commit 8e606a8 into OCA:16.0 May 11, 2023
@pedrobaeza pedrobaeza changed the title [OU-FIX] Base: Add help in ir.actions.act_window translation exclusions [16.0][OU-FIX] Base: Add help in ir.actions.act_window translation exclusions May 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants