-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
base_external_dbsource and import_odbc modules adopted for v8.0 #41
Conversation
…d__ and adopted for instalation on v8.0 : created new form views for both modules as old ones was for v6.1
Thanks @Lauris-v
|
@Lauris-v have you signed the Contributor License Agreement for OCA? http://odoo-community.org/page/website.cla |
@eLBati Fixed PEP8 errors. |
And more PEP8 errors, I guess its time I started to study it carefully. |
@Lauris-v locally you can use tools like |
@eLBati Thanks for info! I already found the Python PEP8 tool and used it to find and correct the style errors. It was good experience for me. |
@Lauris-v it is not necessary, especially since this is not a new addon, but it is something nice to do if you intend on participating in the OCA addons effort :-) |
@gurneyalex Then I will try to sign and send it. |
About the CLA, let me expand a bit what @gurneyalex said: the OCA repos should indeed have only contributors that signed the CLA so that the association can defend itself and the modules legally. For that specific PR, it is a lint change so you are not (yet) marked among the authors, so the CLA is not strictly necessary. But it is still a very good idea. Thanks @Lauris-v! |
@lepistone Thanks for clarification. Now I understand the need of it and I will send it as soon as I get a chance to do so. |
Can't really make a review, but I just wanted to ask if there are plans to integrate those features into https://github.com/OCA/connector/ ? |
👍 |
'raise_import_errors': fields.boolean('Raise import errors', | ||
help="Import errors not handled, intended for debugging purposes." | ||
"\nAlso forces debug messages to be written to the server log."), | ||
help="Import errors not \ |
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'm not a fan of using backslashes, when you can use:
help="spam spam spam spam spam"
"and eggs"
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.
Agree with @veloutin
Also, after a (
you can start a new indented line, instead of aligning the text at the far right. For example:
'raise_import_errors': fields.boolean(
'Raise import errors',
help="Import errors not handled, intended for debugging purposes."
"\nAlso forces debug messages to be written to the server log."),
👍 |
IMO the code could be nicer using a different indentation strategy, but nevertheless 👍 |
This PR cannot be merged due to conflicts. This can be resolved by rebasing with |
@Lauris-v Could you rebase it please ? |
@Lauris-v ping |
Since this PR is accepted but the author hasn't come back with a rebase, I will merge manually. Hopefully the tests will not fail. If they do, I will fix them. |
Moved both modules out of
__unported__
directory and reworked form views for both modules as they were not displayed correctly.No other changes were made because functions were working as supposed.