-
-
Notifications
You must be signed in to change notification settings - Fork 129
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
[UPG] Base migration upgrade for fields rename. #125
base: master
Are you sure you want to change the base?
Conversation
b18e530
to
72db965
Compare
absolute_file_path, replaces, "Change file content of %s" % filename | ||
) | ||
field_renames = renamed_fields.get("replaces") | ||
# To be safe we only rename fields on files associated with the current replaces | ||
if field_renames: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This creates an UnboundLocalError:
module_migration.run()
~~~~~~~~~~~~~~~~~~~~^^
File "/home/data/git/OCA/odoo-module-migrator/odoo_module_migrate/module_migration.py", line 34, in run
migration_script.run(
~~~~~~~~~~~~~~~~~~~~^
self._module_path,
^^^^^^^^^^^^^^^^^^
...<4 lines>...
self._migration._commit_enabled,
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
)
^
File "/home/data/git/OCA/odoo-module-migrator/odoo_module_migrate/base_migration_script.py", line 160, in run
self.process_file(
~~~~~~~~~~~~~~~~~^
root,
^^^^^
...<4 lines>...
commit_enabled,
^^^^^^^^^^^^^^^
)
^
File "/home/data/git/OCA/odoo-module-migrator/odoo_module_migrate/base_migration_script.py", line 230, in process_file
if re.findall(pattern, new_text):
^^^^^^^^
UnboundLocalError: cannot access local variable 'new_text' where it is not associated with a value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've readded the definition of new_text assigned by the call tools._replace_in_file just before the if in L216. It fix the issue, but in any case: how did you ran the code? any specifics that I could add to the unit tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have merged this pr in my own fork. Then used it to migrate some modules from the brand repo.
72db965
to
6b23df0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code review of the fix. No func test yet.
No description provided.