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

Add pass to removed blocks of code #386

Merged
merged 7 commits into from
Aug 26, 2023

Conversation

vthemelis
Copy link
Contributor

Solves #352 more generally.

We saw this in exception handlers but I think that it can manifest itself on any construct that has an expected child block.

We could potentially even totally remove the statements in the test as they are no-op.

@coveralls
Copy link

coveralls commented Aug 26, 2023

Coverage Status

coverage: 97.819% (+0.009%) from 97.81% when pulling 945ddf4 on vthemelis:empty-excpetion-handler into 397ef7d on agronholm:master.

@agronholm
Copy link
Owner

You need to fix the linting error and add a note to the changelog. What's with the gitignore change?

@vthemelis
Copy link
Contributor Author

You need to fix the linting error

Fixed

and add a note to the changelog.

Added

What's with the gitignore change?

I was getting a lot of .coverage.<blah> files when running tox locally. I can remove the change if you prefer but those files are not covered by any of the existing rules and I don't think that they should ever be committed to source control.

.gitignore Outdated Show resolved Hide resolved
docs/versionhistory.rst Outdated Show resolved Hide resolved
@agronholm
Copy link
Owner

agronholm commented Aug 26, 2023

The test you added seems to already pass in master.

EDIT: Never mind, I was on a 3.8 virtualenv so the test didn't get run.

@agronholm agronholm merged commit 068edfd into agronholm:master Aug 26, 2023
7 checks passed
@agronholm
Copy link
Owner

Thanks!

@vthemelis vthemelis deleted the empty-excpetion-handler branch August 27, 2023 00:57
@vthemelis
Copy link
Contributor Author

@agronholm, I opened python/cpython#108521 where they pointed out that maybe we should only be filling in body attributes of AST nodes. Would you like me to raise another PR to fix this?

@agronholm
Copy link
Owner

Sure.

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