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

[14.0][FW] ddmrp: multiple ports from 15.0 #300

Conversation

@OCA-git-bot
Copy link
Contributor

Hi @JordiBForgeFlow, @LoisRForgeFlow,
some modules you are maintaining are being modified, check this out!

@sebalix sebalix force-pushed the oca-port-from-15.0-to-14.0-pr-203-210-212-213-219-232-246-256 branch from 1282b00 to c30c909 Compare July 5, 2023 09:26
@sebalix sebalix force-pushed the oca-port-from-15.0-to-14.0-pr-203-210-212-213-219-232-246-256 branch from c30c909 to 0c9faae Compare July 5, 2023 09:32
@sebalix sebalix marked this pull request as ready for review July 5, 2023 09:32
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.

LG

Copy link
Contributor

@LoisRForgeFlow LoisRForgeFlow left a comment

Choose a reason for hiding this comment

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

Thanks for checking

@@ -1917,7 +1917,7 @@ def action_view_future_adu(self):
@api.model
def cron_ddmrp_adu(self, automatic=False):
"""calculate ADU for each DDMRP buffer. Called by cronjob."""
auto_commit = not getattr(threading.currentThread(), "testing", False)
auto_commit = not getattr(threading.current_thread(), "testing", False)
Copy link
Contributor

Choose a reason for hiding this comment

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

In v14 I can see multiple usages of threading.current_thread(). Are you sure this is a needed backport?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Needed, probably not.
I checked on Python 3.7 / 3.8 / 3.9, and both are equivalent:

Python 3.7.3 (default, Oct 31 2022, 14:04:00) 
[GCC 8.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import threading
>>> threading.currentThread == threading.current_thread
True
Python 3.8.10 (default, May 26 2023, 14:05:08) 
[GCC 9.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import threading
>>> threading.currentThread == threading.current_thread
True
Python 3.9.5 (default, Nov 23 2021, 15:27:38) 
[GCC 9.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import threading
>>> threading.current_thread == threading.currentThread
True

So I guess it's safe to backport it as it's supported on old Python versions (especially the 3.7 which is the one recommended by Odoo I think for v14? I don't remember).
If you prefer to not backport it you tell me!

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair enough, thanks for double checking! I think we can merge as is.

@LoisRForgeFlow
Copy link
Contributor

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 14.0-ocabot-merge-pr-300-by-LoisRForgeFlow-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit b70edac into OCA:14.0 Jul 5, 2023
6 of 7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 84ebeef. Thanks a lot for contributing to OCA. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants