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

[FIX] report_xlsx: Protect import + CamelCase class #68

Merged
merged 1 commit into from
Aug 19, 2016

Conversation

pedrobaeza
Copy link
Member

This includes improvements in the README to avoid to generate modules depending on this one that generates problems:

  • Protect the import to avoid the breakage when you don't have report_xlsx module accessible.
  • Put the name of the class in CamelCase as PEP8 says.

@pedrobaeza
Copy link
Member Author

cc @Tecnativa

@pedrobaeza
Copy link
Member Author

@antespi, please review

@sbidoul
Copy link
Member

sbidoul commented Aug 18, 2016

👍 for the camelcasing of course

Adding the import error in the readme, I'm -0. At the end of the day, all this hackery is needed because we have suboptimal dependency management. When we have OCA/maintainer-quality-tools#343, it should become largely unnecessary (we probably need the same in rundbot)

@lasley
Copy link

lasley commented Aug 18, 2016

👍

@yajo
Copy link
Member

yajo commented Aug 19, 2016

👍 👍 👍

@pedrobaeza
Copy link
Member Author

@sbidoul, not all the people use or will use that deployment strategy, so I prefer to explicitly put this. I merge then.

@pedrobaeza pedrobaeza merged commit 26bf6ef into 8.0 Aug 19, 2016
@pedrobaeza pedrobaeza deleted the 8.0-report_xlsx-readme-fix branch August 19, 2016 11:11
@yajo
Copy link
Member

yajo commented Aug 22, 2016

@sbidoul I think the real solution would be that Odoo did not load uninstalled modules, and thus you could not install them without before parsing __openerp__.py and finding out external_dependencies, but that's not the way it works right now, so having an ImportError just for adding a repo to your addons path is annoying (under some deployment strategies, of course).

@sbidoul
Copy link
Member

sbidoul commented Aug 22, 2016

@yajo That would help yes, but IMO it's not the root cause. The real issue is that people just copy code arround (or git clone it or whatever), where they should use a proper mechanism to install it on the environments including dependencies. That's why I'm working so much on setuptools-odoo and pip... with very good results.

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