-
-
Notifications
You must be signed in to change notification settings - Fork 107
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
[13.0][IMP] intrastat_product - add notedict to some methods #221
[13.0][IMP] intrastat_product - add notedict to some methods #221
Conversation
@pedrobaeza @luc-demeyer @alexis-via The PR was recreated since the old one #206 went stale. I did the same for the PR in the french localization. |
IMO this is ready to be merged. |
Please fw-port it to upper versions: /ocabot merge patch I'll merge the subsequent PRs as soon as this one gets merged. |
Hey, thanks for contributing! Proceeding to merge this for you. |
Congratulations, your PR was merged at 8eb01ac. Thanks a lot for contributing to OCA. ❤️ |
Hi @pedrobaeza this change caused issues on one of our installations due to adding new arguments in the methods. Is this a common practice in the OCA? |
No, it's not, but it wasn't the less bad option due to the incorrect previous situation after Brexit code. See this thread and the source one #206 for details (you should do it before writing, as it's all linked), and all the OCA modules were patched with the new arguments thanks to @jdidderen-noviat diligence. Only custom things outside OCA should be affected, and it's very easy to fix and detect (no silent problems, but direct crash). |
See old PR #206 for the discussions around these changes.
PRs in the localization repositories :