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

Migrate base_import_async to 9.0 #28

Merged
merged 12 commits into from Mar 23, 2016
Merged

Migrate base_import_async to 9.0 #28

merged 12 commits into from Mar 23, 2016

Conversation

sbidoul
Copy link
Member

@sbidoul sbidoul commented Mar 3, 2016

including latests commit from the 8.0 branch

TODO:

  • fix problem with qweb and/or js that prevents the view to load

@pedrobaeza pedrobaeza mentioned this pull request Mar 3, 2016
2 tasks
@sbidoul
Copy link
Member Author

sbidoul commented Mar 3, 2016

Tested with Connector 9.0.1.0.2

@max3903
Copy link
Sponsor Member

max3903 commented Mar 3, 2016

👍 Code review, no test

@lmignon
Copy link
Sponsor

lmignon commented Mar 3, 2016

👍 Code review + test

delimiter=options.get(OPT_SEPARATOR),
quotechar=options.get(OPT_QUOTING))
delimiter=str(options.get(OPT_SEPARATOR)),
quotechar=str(options.get(OPT_QUOTING)))
Copy link
Member

Choose a reason for hiding this comment

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

Couldn'it end up with 'None' as delimiter and quotechar if no option is set?

Copy link
Member Author

Choose a reason for hiding this comment

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

@guewen could be. It's not been an issue in practice so far. I guess it would impact only people using the API, which is kind of undocumented anyway.

Copy link
Member Author

Choose a reason for hiding this comment

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

@guewen note this change is part of the 8.0 forward port. So if that needs changing I'd do it in another PR.

Copy link
Member

Choose a reason for hiding this comment

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

I see, thanks for clarifying

@guewen
Copy link
Member

guewen commented Mar 4, 2016

👍

@atchuthan
Copy link
Member

👍 Code review

@lmignon lmignon merged commit b5ae458 into OCA:9.0 Mar 23, 2016
@sbidoul sbidoul deleted the 9.0-mig-sbi branch March 23, 2016 09:14
tschanzt pushed a commit to camptocamp/connector-interfaces that referenced this pull request Mar 15, 2018
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

7 participants