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 autoflake to .pre-commit-config.yaml to automatically remove unused imports #1637

Merged

Conversation

jasonwbarnett
Copy link
Contributor

@jasonwbarnett jasonwbarnett commented Dec 27, 2022

I observed a lot of ci failures related to unused typing imports being reported by flake8. This change makes it so that these will automatically get cleaned up by the pre-commit.ci bot.

@jasonwbarnett jasonwbarnett force-pushed the ci/automatically-remove-unused-imports branch from f7b2d83 to 0e2950b Compare December 27, 2022 16:26
@jasonwbarnett jasonwbarnett changed the title add autoflake to ci to remove imports add autoflake to .pre-commit-config.yaml to automatically remove unused imports Dec 27, 2022
@Nusnus
Copy link
Member

Nusnus commented Dec 27, 2022

Can you ref/link such a failure please?
One that this change should handle automatically if I understand correctly.

To give me context mostly

Thanks!

@Nusnus Nusnus self-requested a review December 27, 2022 16:36
@auvipy auvipy self-requested a review December 27, 2022 16:57
@jasonwbarnett
Copy link
Contributor Author

jasonwbarnett commented Dec 27, 2022

Can you ref/link such a failure please? One that this change should handle automatically if I understand correctly.

To give me context mostly

Thanks!

@Nusnus It's hard because I don't know how to find past pre-commit.ci pipelines, but here is one I had to change by hand, a9f357f in #1631

@jasonwbarnett jasonwbarnett force-pushed the ci/automatically-remove-unused-imports branch 3 times, most recently from 0ee9031 to 3af7bbc Compare December 27, 2022 17:32
rev: v2.0.0
hooks:
- id: autoflake
args: ["--in-place", "--imports"]
Copy link
Member

Choose a reason for hiding this comment

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

-i, --in-place make changes to files instead of printing diffs

Cool

--imports IMPORTS by default, only unused standard library imports are removed; specify a comma-separated list of additional modules/packages

What does the --imports here is supposed to affect, based on the above doc?
The default should work without en empty --imports, no?

--remove-unused-variables
remove unused variables

This might also be interesting, but I'm not 100% sure.

@@ -1341,7 +1341,6 @@ class SentinelManagedSSLConnection(
SSL Connection.
"""

pass
Copy link
Member

Choose a reason for hiding this comment

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

kombu/transport/redis.py:1346:1: E303 too many blank lines (3)

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll start working through this now :)

Thanks for pointing out.

@jasonwbarnett jasonwbarnett force-pushed the ci/automatically-remove-unused-imports branch from 0defa9e to ed0fe8c Compare December 27, 2022 18:00
@jasonwbarnett jasonwbarnett requested review from Nusnus and removed request for auvipy December 27, 2022 18:00
@auvipy
Copy link
Member

auvipy commented Dec 27, 2022

pre commits are passing now

@auvipy
Copy link
Member

auvipy commented Dec 27, 2022

but the lint CI is failing

@auvipy auvipy merged commit 6593a2d into celery:main Dec 27, 2022
@jasonwbarnett jasonwbarnett deleted the ci/automatically-remove-unused-imports branch December 27, 2022 18:49
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.

None yet

3 participants