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

added @functools.wraps(run_func) to task_complete_check_wrapper #337

Merged
merged 1 commit into from
Dec 28, 2023

Conversation

mski-iksm
Copy link
Contributor

@mski-iksm mski-iksm commented Nov 21, 2023

I've added @functools.wraps not to change wrapped function's name.

@Hi-king
Copy link
Member

Hi-king commented Dec 28, 2023

LGTM!

@yokomotod
Copy link
Collaborator

yokomotod commented Dec 28, 2023

@mski-iksm Can we have any kind of unit test ?

I'm not sure what problem happen if decorate function name changed. (but yes, sounds problem 😅

I's great if we have concrete problem case and confirm it is prevented

@mski-iksm
Copy link
Contributor Author

@yokomotod
This PR is not to fix any bug, instead it is for readability.

So there is no concrete problem case, and therefore I think no test is required.

@yokomotod
Copy link
Collaborator

I see, then I agree to not have unit test for @functools.wraps functionality itself !

and +1 for using @functools.wraps habit

@Hi-king Hi-king merged commit f5f3956 into m3dev:master Dec 28, 2023
5 checks passed
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.

3 participants