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

[IMP] update_module_names: Add merge option #63

Merged
merged 1 commit into from
Dec 27, 2016

Conversation

pedrobaeza
Copy link
Member

With these minimal changes, we allow to reallocate all the resources related to a module that has been merged into others in later versions. For example, email_template in version 8 that has been integrated in mail module in version 9.

@pedrobaeza
Copy link
Member Author

cc @Tecnativa

@pedrobaeza
Copy link
Member Author

Travis error unrelated

@pedrobaeza
Copy link
Member Author

@moylop260, do you have any tip why this fails?

@moylop260
Copy link
Contributor

Maybe is a flake8-3.2.1 error

flake8 OCA/openupgradelib
Traceback (most recent call last):
  File "flake8", line 11, in <module>
    sys.exit(main())
  File "flake8/main/cli.py", line 16, in main
    app.run(argv)
  File "flake8/main/application.py", line 322, in run
    self._run(argv)
  File "flake8/main/application.py", line 306, in _run
    self.run_checks()
  File "flake8/main/application.py", line 243, in run_checks
    self.file_checker_manager.start(files)
  File "/flake8/checker.py", line 371, in start
    self.make_checkers(paths)
  File "/flake8/checker.py", line 285, in make_checkers
    if argument == filename or should_create_file_checker(filename)
  File "/flake8/checker.py", line 412, in __init__
    self.display_name = self.processor.filename
AttributeError: 'NoneType' object has no attribute 'filename'

@moylop260
Copy link
Contributor

It could be related with https://gitlab.com/pycqa/flake8/issues/272

@pedrobaeza
Copy link
Member Author

It seems so. Thanks for the info. Is in our hands to repair it?

@moylop260
Copy link
Contributor

Options

  1. Wait the next releases
  2. Freeze a old stable version in pip install command

@pedrobaeza
Copy link
Member Author

OK, I think we should choose option 2, isn't it? We did the same on MQT if I don't remember bad.

@moylop260
Copy link
Contributor

This project is not using MQT
Use directly flake8 command from .travis.yml

@pedrobaeza
Copy link
Member Author

Yeah, I know, that's why I tell to do the same.

@moylop260
Copy link
Contributor

We have freeze pylint version from pylint-odoo but flake8 is not freeze

@moylop260
Copy link
Contributor

FYI using
flake8 --version
2.4.1 (pep8: 1.7.0, mccabe: 0.5.2, pyflakes: 0.8.1) CPython 2.7.10 on Darwin

flake8 openupgradelib/
the result was:

openupgradelib/openupgrade_tools.py:31:1: E302 expected 2 blank lines, found 1
openupgradelib/openupgrade.py:101:5: F811 redefinition of unused 'RegistryManager' from line 58

@hbrunn
Copy link
Member

hbrunn commented Dec 16, 2016

what do you think about having a new function to merge module A into B instead? This way, we can account for situations where multiple modules are replaced by another one, and the execution path will be a lot less complicated

@pedrobaeza
Copy link
Member Author

pedrobaeza commented Dec 16, 2016

I preferred to use the same method for not duplicating code. You can still perform the merge of multiple modules into one:

update_module_names([('B', 'A'), ('C', 'A',), ('D', 'A')], merge_modules=True)

With these minimal changes, we allow to reallocate all the resources
related to a module that has been merged into others in later
versions. For example, email_template in version 8 that has been
integrated in mail module in version 9.
@pedrobaeza pedrobaeza force-pushed the update_module_names-merge_option branch from 291963e to 181c1cb Compare December 27, 2016 10:36
@pedrobaeza
Copy link
Member Author

This is finally working as I have fixed Travis.

@hbrunn @yajo @cubells, can any of you please check?

@pedrobaeza
Copy link
Member Author

As we have already tested this feature and need it for a lot of current PRs, I concede myself the luxury of merging it.

@pedrobaeza pedrobaeza merged commit 7f92549 into OCA:master Dec 27, 2016
@pedrobaeza pedrobaeza deleted the update_module_names-merge_option branch December 27, 2016 13:34
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.

None yet

4 participants