Add translations bump - Githubissues

OCA / odoo-module-migrator

Python library to migrate odoo base code from a version to another

GNU Affero General Public License v3.0

123 stars 112 forks source link

Add translations bump #56

Closed baimont closed 3 years ago

baimont commented 3 years ago

This pr allows to change automatically version in translation files (in folder i18n)

baimont commented 3 years ago

Hi. Thanks. for this nice addition.

1. I think your code is to generic. If I understand correctly, it will replace "12.0" pattern by "14.0" text in all i18n files so it could maybe replace undesired text. I think if you add the pattern of `Project-Id-Version` it will restrict the replacement. What do you think ?

2. Could you add test ?

Otherwise, LGTM. Thanks !

Hello @legalsylvain and thanks for your comment.

Indeed it is too generic. I agree with your suggestion.
I'll try :-D

legalsylvain commented 3 years ago

regarding 2.0 it's quite easy. just :

add a "init" file like tests/data_template/module_080/i18n/fr.po with a sample translation file. (you can add a text like 8.0 in a translation for exemple, to be sure it is not replaced.
add a "end" file like tests/data_result/module_080_130/i18n/fr.po with the attended translation file.

yelizariev commented 3 years ago

@legalsylvain apparently there is no way to mark migration script as optional.

When I need to disable something, I make a patch and complie the tool via https://github.com/acsone/git-aggregator

legalsylvain commented 3 years ago

@yelizariev : I don't get your point. I was thinking to add an option --bump-po-file for the odoo-module-migrator tool, disabled by default. Like other options described here : https://github.com/OCA/odoo-module-migrator#available-arguments

yelizariev commented 3 years ago

@legalsylvain existing options are kind of hardcoded and not part of migration_scripts folder.

What I mean is that if we add new script in the folder, e.g. odoo_module_migrate/migration_scripts/python_scripts/migrate_allways/bump_version_i18n.py

then there is no way to disable that file only without changing core files (e.g. odoo_module_migrate/migration.py)

legalsylvain commented 3 years ago

@yelizariev I see. We could imagine a --disabled-script-names=bump_version_i18n,other_script_name, etc... but it should be implemented.

Well, let's wait @baimont answer, regarding the reason of this PR first ;-)

baimont commented 3 years ago

Well, let's wait @baimont answer, regarding the reason of this PR first ;-)

It's already here https://github.com/OCA/odoo-module-migrator/pull/56#discussion_r672280799 @legalsylvain ^^

and I agree with @pedrobaeza on https://github.com/OCA/odoo-module-migrator/pull/56#discussion_r672281895

legalsylvain commented 3 years ago

It's already here #56 (comment) @legalsylvain ^^

sorry. didn't saw !

If it's controversial, I guess it'd better stay a customization for me :-D

OK. closing then.

Thanks @pedrobaeza for your insight on that topic.