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

Fix removed comments in stub files #3745

Merged
merged 6 commits into from Jul 9, 2023
Merged

Conversation

JelleZijlstra
Copy link
Collaborator

@JelleZijlstra JelleZijlstra commented Jun 23, 2023

Fixes #1239. Fixes #1020.

I believe we can fix this without going through the preview style as we're only changing code that was not yet formatted with black; https://black.readthedocs.io/en/stable/the_black_code_style/index.html#stability-policy explicitly calls out that we can fix bugs where we remove comments.


def h():
...
# bye
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Arguably we could turn this into def h(): ... # bye, but that's more complicated to implement and if the user put it on a separate line, we might as well keep it there.

@github-actions
Copy link

github-actions bot commented Jun 23, 2023

diff-shades reports zero changes comparing this PR (238b176) to main (0b4d7d5).%0A%0A---%0A%0AWhat is this? | Workflow run | diff-shades documentation

Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

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

Looks great!

@hauntsaninja hauntsaninja merged commit 257d392 into psf:main Jul 9, 2023
35 checks passed
@hauntsaninja
Copy link
Collaborator

I guess it could make more sense for the testing to look at ".pyi" extension instead of "_pyi" stem, but whatever

@JelleZijlstra JelleZijlstra deleted the stubignore branch July 9, 2023 23:04
@JelleZijlstra
Copy link
Collaborator Author

I think I tried that briefly but there was a fairly deep assumption in the test framework that everything has a .py extension, so I used this approach instead. Agree that solution would be cleaner though.

@ichard26
Copy link
Collaborator

Totally out of the loop, but would you think it would be better in the long run to migrate away from assuming everything is a .py file? If so, let's open an issue.

@JelleZijlstra
Copy link
Collaborator Author

Yes, let's do that

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.

Comments removed in pyi files INTERNAL ERROR: *.pyi file + function + comment + ellipsis
3 participants