Apply PEP-7 to Python/import.c
#29
Conversation
|
The base branch went wrong ! |
|
I think this one should be applied at the end of my chain of PR. All other PRs can be merged sequentially but this one causes a conflict, so it's better to make it last. |
|
FTR, I need to update the tests: |
DinoV
left a comment
DinoV
left a comment
There was a problem hiding this comment.
Just a couple of small nits but otherwise LGTM
|
|
||
| PyObject *globals = PyEval_GetGlobals(); | ||
|
|
||
| if (PyMapping_GetOptionalItem(lz->lz_builtins, &_Py_ID(__import__), &import_func) < 0) { |
There was a problem hiding this comment.
This open bracket should be on the same line?
There was a problem hiding this comment.
Well... strictly speaking, the entire call should be wrapped weirdly according to PEP-7 but we're not exceeding the line for too much so I'm applying the principle of "let's not be nitpick" here. I'm just making sure that the code doesn't exceed too much the 80 chars width (otherwise I really need to scroll and it's a bit annoying I think for readers)
|
After checking a bit why the test actually failed, it's because the message was like this since 2010. So I won't change "Empty module" to "empty module" (I thought it was a new message). |
|
@DinoV Actually, maybe you prefer to hold that PR in case something else changes in the main branch? I can definitely update it when nothing more needs to be added so that I only need to reformat the code once. WDYT? |
|
Let's get this in now. If we have merge conflicts with these changes we'd have them without these changes as well, and if we leave this pending we'd have two chances of getting merge conflicts :) |
4 participants
No description provided.