Imposing "correct" isort as requirement for a green pipeline #73
Conversation
| if: ${{ failure() }} | ||
| run: | | ||
| echo "::notice::In project root run 'python.exe -m isort .' and commit changes to fix errors." | ||
| exit 1 |
There was a problem hiding this comment.
Is failure the right thing to do? Maybe stylistic issues should only be reported as warnings, like it is done with codecov reports?
original flake8 line for ER has
--exit-zero
so errors are treated as warnings
There was a problem hiding this comment.
We can do it differently. The reason why I decided to go for an exit 1 of the echo step is an attempt to make it more obvious that the user might want to consult the produced statement that contains a proposed fix to the issue.
The pipeline itself has already failed in the preceding step.
We should probably discuss.
| [tool.hatch.build.targets.wheel] | ||
| packages = ["EasyReflectometry"] | ||
| [tool.isort] | ||
| line_length = 88 |
There was a problem hiding this comment.
This is probably too conservative. With hi res monitors, we tend to write long lines, especially for message formatting.
We specify 120 as the standard max length.
Any good reason to limit the width to 88 (apart from this being the standard in black?)
Likewise, the original flake8 line for this project lists
--max-line-length=127
which, while being inconsistent with the EasyScience standard ( 😅 ) is less restrictive than 88
There was a problem hiding this comment.
It was just a value that frequently showed up.
Monkey see, monkey do ;)
The CI pipeline will now fail if python imports are done "wrongly" according to isort.
The requirements for the
isortsorting are defined inpyproject.tomlIt is also possible to get the requirements applied on file save in VSCode by using the
settings.jsonfile invscode_template.To help the user in case of a failing pipeline the following now step echo's a potential resolution to the error. This is not an optimal solution, but it is considered good enough for now.
Bumped: