-
Notifications
You must be signed in to change notification settings - Fork 75
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
Turn on remove-all-unused-imports
in for autoflake
#414
Turn on remove-all-unused-imports
in for autoflake
#414
Conversation
@zliang-akamai This PR looks great! I do have one concern: given this is a package intended to be consumed by users would this accidentally break any indirect imports? Most of the dropped imports seem pretty insignificant but it might be worth double-checking just in case 🙂 |
@lgarber-akamai Thanks for the reminder! I think all imports it removed is not relevant to the module it's in. For example, The right way to import I doubt there is any indirect import like that, and even that's the case, they can easily fix it by changing the import path. Autoflake would ignore (not to remove) any unused import in But we can definitely mark it as a breaking change as a head up to the users. |
Sounds good to me! I think this is a very positive change, we just need to make sure we communicate it well 👍 |
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 +1 to communication.
📝 Description
To cleanup unused imports
✔️ How to Test