Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
gh-104469 Convert_testcapi/vectorcall.c to use AC#106557
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
littlebutt commented Jul 9, 2023
- issue: Improve the readability and maintainability of test_capi using the AC #104469
bedevere-bot commented Jul 9, 2023
Most changes to Python require a NEWS entry. Please add it using the blurb_it web app or the blurb command-line tool. |
ghost commented Jul 9, 2023 • edited by ghost
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by ghost
Uh oh!
There was an error while loading. Please reload this page.
sobolevn left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that we need to convert test_ prefixed functions. They are executed as tests directly.
corona10 commented Jul 9, 2023
Well, I am okay with it. We already did. |
corona10 left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First thing, method names are wrong.
Please change method names and then executing the make clinic
Modules/_testcapi/vectorcall.c Outdated
| } | ||
| /*[clinic input] | ||
| _testcapi.test_pyobject_fastcalldict |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| _testcapi.test_pyobject_fastcalldict | |
| _testcapi.pyobject_fastcalldict |
Modules/_testcapi/vectorcall.c Outdated
| } | ||
| /*[clinic input] | ||
| _testcapi.test_pyobject_vectorcall |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| _testcapi.test_pyobject_vectorcall | |
| _testcapi.pyvectorcall_call |
Modules/_testcapi/vectorcall.c Outdated
| _testcapi.test_pyobject_vectorcall | ||
| func: object | ||
| func_args: object | ||
| kwnames: object = NULL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| kwnames: object=NULL | |
| kwnames: object=None |
Modules/_testcapi/vectorcall.c Outdated
| } | ||
| /*[clinic input] | ||
| _testcapi.test_pyvectorcall_call |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| _testcapi.test_pyvectorcall_call | |
| _testcapi.pyvectorcall_call |
bedevere-bot commented Jul 9, 2023
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
bedevere-bot commented Jul 9, 2023
Most changes to Python require a NEWS entry. Please add it using the blurb_it web app or the blurb command-line tool. |
littlebutt commented Jul 9, 2023
@corona10 Thank you for your suggestions and I fixed these method names. But the complaints still exist because of missing arguments in test cases. |
corona10 commented Jul 9, 2023
According to CI, it looks not, did you rebuild the code? |
littlebutt commented Jul 9, 2023
No, I didn't. I just ran |
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Dong-hee Na <donghee.na92@gmail.com>
corona10 left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm