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

[ADD] missing-import-error, missing-manifest-dependency #68

Merged
merged 1 commit into from
Sep 26, 2016

Conversation

moylop260
Copy link
Collaborator

@moylop260 moylop260 commented Sep 24, 2016

Based on external dependencies guideline

Inspired from OCA/maintainer-tools#224

FYI the variable DFLT_IMPORT_NAME_WHITELIST was filled running this check on odoo/odoo#master..odoo/odoo#8.0

PYTHONPATH=~/build/OCA/pylint-odoo pylint --msg-template="{path}:{line}: [{msg_id}({symbol}), {obj}] {msg}" -d all -e missing-import-error --load-plugins=pylint_odoo odoo/addons/* odoo/openerp/addons/* odoo odoo/openerp -rn

# self-odoo
'odoo', 'openerp',
# Known external packages of odoo
'PIL', 'babel', 'dateutil', 'decorator', 'docutils', 'jinja2', 'ldap',
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be caught from the requirements.txt of Odoo?

Copy link
Collaborator Author

@moylop260 moylop260 Sep 24, 2016

Choose a reason for hiding this comment

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

That was my first option

  • But for many cases the pip install package is different to import package

But I have extracted the packages running this check from odoo 8.0, 9.0 and master:
PYTHONPATH=~/build/OCA/pylint-odoo pylint --msg-template="{path}:{line}: [{msg_id}({symbol}), {obj}] {msg}" -d all -e missing-import-error --load-plugins=pylint_odoo odoo/addons/* odoo/openerp/addons/* odoo odoo/openerp -rn

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I didn't see this. Anyways, we need to control a different list depending versions. What about including this on the specific cfg for each version?

Copy link
Collaborator Author

@moylop260 moylop260 Sep 26, 2016

Choose a reason for hiding this comment

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

(Don't worry.)

Yes, that is the intention.
Here we have a "Default" value for initial assignation from configuration file.
But we can create a configuration file by each version with this value but is from running step MQT configuration files of pylint

@@ -229,15 +259,71 @@ def check_odoo_relative_import(self, node):
self.add_message('odoo-addons-relative-import', node=node,
args=(self.odoo_module_name))

@utils.check_messages('odoo-addons-relative-import')
@staticmethod
def _is_absoulte_import(node, name):
Copy link
Member

Choose a reason for hiding this comment

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

s/absoulte/absolute

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed
thanks!

@moylop260 moylop260 force-pushed the master-oca-whlst-pack-moy3 branch from 162f7b1 to 90739a6 Compare September 24, 2016 01:03
@yajo
Copy link
Member

yajo commented Sep 24, 2016

👍

1 similar comment
@bealdav
Copy link
Member

bealdav commented Sep 24, 2016

👍

@moylop260 moylop260 changed the title [ADD] openerp-exception-warning, missing-manifest-dependency [ADD] missing-import-error, missing-manifest-dependency Sep 24, 2016
@moylop260 moylop260 force-pushed the master-oca-whlst-pack-moy3 branch 2 times, most recently from d3dd205 to e06171e Compare September 24, 2016 15:02
Copy link

@alan196 alan196 left a comment

Choose a reason for hiding this comment

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

👍

@moylop260
Copy link
Collaborator Author

Could we merge it?

@pedrobaeza
Copy link
Member

pedrobaeza commented Sep 26, 2016

You didn't answer me about the possibility of extracting known libraries from requirements.txt so that we can correctly parse them when this changes through versions.

@moylop260
Copy link
Collaborator Author

Are you talking about my last answer #68 (comment)?

@pedrobaeza
Copy link
Member

Thanks as always for your good job.

@pedrobaeza pedrobaeza merged commit 8e60b4c into OCA:master Sep 26, 2016
@moylop260 moylop260 deleted the master-oca-whlst-pack-moy3 branch September 26, 2016 22:42
@moylop260
Copy link
Collaborator Author

Thanks for your review, feedback and merge!

moylop260 pushed a commit to vauxoo-dev/pylint-odoo that referenced this pull request Sep 26, 2016
[ADD] missing-import-error, missing-manifest-dependency
moylop260 pushed a commit to vauxoo-dev/pylint-odoo that referenced this pull request Sep 26, 2016
[ADD] missing-import-error, missing-manifest-dependency
moylop260 added a commit to Vauxoo/pylint-odoo that referenced this pull request Sep 26, 2016
* Merge pull request OCA#68 from vauxoo-dev/master-oca-whlst-pack-moy3

* [REF] import-error: Increase test expected
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants