Question: Why use zero return code when path is clearly missing?

[gangefors]

Running isort --check and forgetting the path option have isort returning 0. But this also makes any CI job that runs this command return green which I highly doubt anyone wants.

Is there a reason isort returns 0 when it clearly didn't get the correct input?

My resoning is that if I tell it to --check something I would expect it to return non-zero if I fail to provide it with something to check. I would however expect a 0 return code if I provide a path where there happens to be no .py files for it to check.

[gangefors]

That was an extremely fast response. And your commit answers my question. It would be better to have a non-zero return code when no files are passed together with options.

Thanks for all the work you put in on this excellent software.

[timothycrosley]

Hi @gangefors,

Thanks for reporting this issue! I have to admit that this wasn't an intentioned decision, but rather an oversight. Left over from the days when isort didn't strictly need files passed in to action. I believe the correct behaviour is to error in the case where any arguments are passed in via the CLI but no paths or streaming content is. I've pushed this out in a hot fix release[0], and added additional test cases to ensure the new behaviour persists.

[0] -https://timothycrosley.github.io/isort/CHANGELOG/#522-july-30-2020

Thanks!

~Timothy

[mikeshardmind]

This causes shell expansions that are empty and passed to isort to error, requiring checking the expansion for its results before using with isort now.

[timothycrosley]

@mikeshardmind thanks for the note!

This sounds to me like this is an improvement, as it encourages explicit usage. However, that means this should have been done as a minor release and not a patch release.

To clarify: Does this hurt your workflow, or do you expect it to be a long term detriment to other's workflows?

[mikeshardmind]

@timothycrosley I found out about this issue through someone pointing out it broke a workflow.

It's also not necessarily uncommon to have shell scripts as part of CI in order to only check changed files (for stylistic checks anyhow), so in projects with mixed source code types, if none of the python files have changed, this may break CI

[timothycrosley]

That makes a lot of sense! And thank you for making that note here so if others run into that issue they know how to resolve it. Wanted to make sure it wasn't something I would need to immediately revert or revert in the near future, now that I have a fuller picture of the implications I think it was a good change over all but should have been delayed for the next minor. Still, this was a mistake in my part to push a breaking change out as a patch release, I'll be more cognizant of that in the future.