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

[17.0] [MIG] report_py3o: Migration to 17.0 #931

Open
wants to merge 135 commits into
base: 17.0
Choose a base branch
from

Conversation

bug2point0
Copy link

No description provided.

lmignon and others added 30 commits September 11, 2024 11:16
Replace README.md by README.rst (not finished)
Remove <data> in views
Protect import of py3o libs
Remove dep on base module
Other small changes
Rename __openerp__.py to __manifest__.py
OCA-git-bot and others added 25 commits September 11, 2024 11:17
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: reporting-engine-16.0/reporting-engine-16.0-report_py3o
Translate-URL: https://translation.odoo-community.org/projects/reporting-engine-16-0/reporting-engine-16-0-report_py3o/
Currently translated at 100.0% (46 of 46 strings)

Translation: reporting-engine-16.0/reporting-engine-16.0-report_py3o
Translate-URL: https://translation.odoo-community.org/projects/reporting-engine-16-0/reporting-engine-16-0-report_py3o/es/
Currently translated at 100.0% (46 of 46 strings)

Translation: reporting-engine-16.0/reporting-engine-16.0-report_py3o
Translate-URL: https://translation.odoo-community.org/projects/reporting-engine-16-0/reporting-engine-16-0-report_py3o/es_AR/
Currently translated at 17.3% (8 of 46 strings)

Translation: reporting-engine-16.0/reporting-engine-16.0-report_py3o
Translate-URL: https://translation.odoo-community.org/projects/reporting-engine-16-0/reporting-engine-16-0-report_py3o/it/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: reporting-engine-16.0/reporting-engine-16.0-report_py3o
Translate-URL: https://translation.odoo-community.org/projects/reporting-engine-16-0/reporting-engine-16-0-report_py3o/
Currently translated at 100.0% (46 of 46 strings)

Translation: reporting-engine-16.0/reporting-engine-16.0-report_py3o
Translate-URL: https://translation.odoo-community.org/projects/reporting-engine-16-0/reporting-engine-16-0-report_py3o/it/
Currently translated at 100.0% (46 of 46 strings)

Translation: reporting-engine-16.0/reporting-engine-16.0-report_py3o
Translate-URL: https://translation.odoo-community.org/projects/reporting-engine-16-0/reporting-engine-16-0-report_py3o/sv/
Copy link
Member

@angelmoya angelmoya left a comment

Choose a reason for hiding this comment

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

👍 LGTM tested on runbot ok

"depends": ["web"],
"external_dependencies": {
"python": ["py3o.template", "py3o.formats"],
"deb": ["libreoffice"],
Copy link
Member

Choose a reason for hiding this comment

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

More info: https://github.com/odoo/odoo/blob/d759264c6e9b7a8cc8b6cffdc5509df654cd434c/odoo/modules/module.py#L508

Suggested change
"deb": ["libreoffice"],
"bin": ["libreoffice"],

I think this should be changed on all versions... and other modules...

Copy link
Author

@bug2point0 bug2point0 Oct 16, 2024

Choose a reason for hiding this comment

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

odoo docs are pretty clear on this. Nice catch.

Copy link
Member

@Tardo Tardo Oct 16, 2024

Choose a reason for hiding this comment

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

Umm... looks like OCA "unofficial" way (i don't know where is documented) to handle dependencies in CI env: https://github.com/OCA/oca-ci/blob/c10c90835d3baf6576faaae03f593099b9995009/bin/oca_install_addons#L16

Perhaps a solution could be to use bin to have the Odoo check and deb to indicate how to install the package with the 'apt package manager'?

ping @sbidoul

Copy link
Member

Choose a reason for hiding this comment

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

You can add bin if you want, but please keep deb so libreoffice is installed in ci and runboat.

That said, I'm not sure libreoffice is a hard dependency. IIRC it's optional when using report_py3o_fusion_server, so in that case we should not add bin.

Copy link
Member

Choose a reason for hiding this comment

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

@bug2point0 I think there should be both “bin” and “deb”.
As I see it, actually it all depends on which module you give more importance to 'report_py3o_fusion_server' vs 'report_py3o'... Everything else is a mess of dependencies (if you choose X with Y but you have Z then disable W... :/). At least from what I could see.

You decide, if you choose only “deb” it seems right to me... and if you choose “bin” and “deb” the same. Little more I can say.

Co-authored-by: Alexandre Díaz <[email protected]>
@sersanchus
Copy link

sersanchus commented Oct 16, 2024

I think I've found an issue. There is a demo report for res.users model that is defined in the file 'demo/res_users_report_py3o.xml'. However, it is not added in the 'demo' section of the '__manifest__.py' but rather in 'data'. Therefore, it is always installed even if demos are not required for the module.

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.