-
Notifications
You must be signed in to change notification settings - Fork 82
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
load config from pyproject.toml or setup.cfg #79
Conversation
@hakancelik96 Please review again |
Made the @hakancelik96 Please review again Note that the test failure is due to an unrelated feature break in Python 3.10.0a0 |
@hakancelik96 any thoughts? |
I am not the maintainer of this project, sorry. I just gave a little suggestion. I am developing a similar project named unimport |
@hakancelik96 I see. Thanks for sharing! I'll have a look. @myint Have you got any feedback/other thoughts? I think that not having the ability to load configuration from a file is a real showstopper nowadays. |
@myint have you any feedback or time to merge? |
I'm sorry to ping you like this @myint. I know how open source can be energy draining and I definitely understand if you don't have time to look into this PR. However, is it possible to know if I should go ahead and publish the changes of this PR under a different name on PyPI so that I can use it? Thanks a lot! |
Is there just 1 person with merge permission for this repo? Maybe time to assign others? Seems like a waste of work not to get this in. |
@sigmavirus24 Any thoughts on who should be added as additional maintainers? I think the other PyCQA owners would have appropriate privileges now that I've moved this and |
I haven't been involved in this project so I don't know which contributors should be invited. Other owners on PyCQA tend to not merge things across other projects they're not actively working on. Are there other folks in PyCQA that have worked on these projects you'd like to invite? Also, my response time is going to be super slow with a 3 week old that I'm caring for
On this topic, someone sent a pull request and did work, yes. I've not evaluated it, but just because someone did work, does not mean it should ever just be accepted or merged because they did it. Projects need to guard their boundaries more and more because maintainer burnout is an increasingly prevalant problem and throwing new people into the fire doesn't ever improve things. All new feature work has to be carefully criticized else a project become so large, unruly, and complex that it's functionally unmaintainable. |
Fwiw, the changes in this PR worked flawlessly for me. pip install https://github.com/akeeman/autoflake/archive/patch-1.zip
autoflake **/*.py Did nothing at first (as expected) but after adding the following to
The only thing I noticed (which I think is fine) is that only snake case is allowed. Some tools use param case in
raised |
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.
HI @akeeman, thank you very much for contributing! Are you still interested in getting this merged? If so, can you update your branch and address the comment on six?
Rebased |
Let me fix #79 (comment) soon... |
Doesn't anyone know some configuration (file) abstraction layer? Because it's insane to write this logic over and over again for different libraries... |
The idea is that it should match the flags!
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.
Thank you very much for contributing. I'm prepping a release, so I went ahead and addressed some of the comments to get the PR ready to go!
@fsouza @akeeman I notice that the way this is implemented, values from config files take precedence over command line flags. That behaviour is surprising and at odds with the UNIX convention where the order of precedence is command line flags > environment variable > user config files > system config files. Was that a deliberate choice? |
No, that one slipped. Sorry about that, let me send a fix. |
Simple implementation for the currently missing functionality to load configuration from certain files:
to do: