Skip to content

Conversation

@grindtildeath
Copy link

Supersedes: #75 (as author repository doesn't exist anymore)

Copy link

@simahawk simahawk left a comment

Choose a reason for hiding this comment

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

code review, only on migration + fix commits. LG.

@leemannd
Copy link

Hello @sebastienbeau do you have time to give us a feedback about this?

@rvalyi
Copy link
Member

rvalyi commented Oct 16, 2019

I don't want to anticipate @sebastienbeau response, but sadly Akretion skipped Odoo 11.0 version entirely (it's quite counter productive to try to be expert in every version) so we easily review PRs against 10.0 and 12.0, but for 11.0 it's much less likely...

Copy link

@leemannd leemannd left a comment

Choose a reason for hiding this comment

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

Functional testing only.
@rvalyi I totally undersdand your point. Thanks for the quick answer

@grindtildeath
Copy link
Author

@rvalyi @sebastienbeau If you're using this module in v12.0, you might want to refactor this part:
https://github.com/akretion/odoo-usability/blob/12.0/account_invoice_update_wizard/models/account_invoice.py#L12-L21

This was probably done to avoid having the wizard overwrite the changes done manually when calling its action (as create is called from the UI, the values were overwritten by default_get), but IMO the solution here is cleaner.

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.

8 participants