flake8-isort
flake8-isort copied to clipboard
gforcada
Honor flake8's own --config parameter
In reference to #42
Within parse_options, options.config should be the command-line parameter received by flake8 itself. Once you have that, it's just matter of giving it proper priority.
Note that this quick implementation does not check if the provided file actually contains a valid configuration. While thinking about it, I'm not certain whether that should be an immediate error, or whether it should keep looking for a valid configuration elsewhere...
@SwampFalc sorry for not replying nor noticing this pull request sooner :confounded:
I just merged an even older pull request, would it be possible to refactor your code on top of the master changes that just landed? :sweat: sorry that you have to go on extra lengths to contribute your changes!
Hmmm... I'm not sure to what degree those changes are compatible with mine...
I mean, we're talking about two separate tools (isort and flake8) that are brought together by this one and both those tools might already have a configuration file.
The other pull request basically starts from the assumption that there's an existing isort configuration. Mine was based more on the assumption that there's an existing flake8 configuration. Which one of these situations is most likely? I don't really know off the top of my head...
So I think the ball is in your camp since this is your project: you now load configuration through isort. Do you also want to potentially load configuration through flake8?
Do you want one of those modes to be the default? (ie. what sort of parameters would be required)
Do you want to be able to combine both sources of configuration?
I'm certainly not against the idea of spending some more time on this project, but it would be a bit pointless to start work without answers to these questions.
@SwampFalc completely valid questions and points :100:
Right, I'm quite swamped with job and family (and mostly both at the same time :sweat_smile: ) so I didn't really made out what was exactly you were trying to achieve here, sorry for that :raising_hand_man:
As this is a plugin for a tool (flake8) to bring functionality from another tool (isort) I think that the direction we took with #79 should be the way to go: ensure that isort on its own can find the configuration and then flake8 (via this plugin) will also find it as well.
I can see the use case of having an extra configuration file, but what's the benefit of having to pass it explicitly if on your CI/while working/etc one wants to run the tools and check their output? :thinking:
I'm deeply sorry that you spend your time crafting this pull request and now I'm changing a bit the project direction :bow:
I'm also happy to continue discussing this matter if there is still interest in getting it to work. As you mentioned, the default could be #79 i.e. let isort get the configuration, but if that configuration is given by flake8 itself, then override that decision? If the code doesn't get as bloated as I end up creating it, it might be possible to keep both workflows.
First of all, don't worry too much, I think I spent 15 minutes on it.
One question I have (and perhaps @jnns could chime in here?) is why someone would want to run both standalone isort and flake8-isort.
I mean... If you run isort itself, why would you run flake8-isort afterwards? To verify that isort did it's job? If you're sharing configuration, it's really not going to give you a second opinion...
The other way round makes a bit more sense: run flake8-isort to see if anything's wonky and then either ignore it, fix it manually, or run isort. But wouldn't it be simpler to just run isort anyway, in that last case?
My take on that, though you asked @jnns, is CI: it does not make any sense to run isort there, as you will not commit the changes, but flake8-isort will warn you, and then locally you can run isort.
Specially when collaborating on repositories, is a perfect way to enforce the same code style (as black does not reformat imports :slightly_smiling_face: ).