Skip to content
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

Lint #1093

Merged
merged 15 commits into from
Oct 13, 2023
Merged

Lint #1093

merged 15 commits into from
Oct 13, 2023

Conversation

marcus-oscarsson
Copy link
Member

Linted and formatted all files.

  • flake8 now passes.
  • Excluding the test directory for the time being

@fabcor-maxiv
Copy link
Contributor

What are all the # noqa things? Can't we be more specific with flake8 (or whatever the linter is)?

With every linter I use (as far as I can recall), it is possible to be explicit by specifying which linting rule we want to make an exception for. Which I believe is much better for the code quality. By disabling linting completely for the whole line we could hide some actual useful linting warning/errors. It is not proper style from my point of view to disable the whole linter for a line.

Seems like it is possible:
https://flake8.pycqa.org/en/latest/user/violations.html#in-line-ignoring-errors

For example:

example = lambda: 'example'  # noqa: E731

@marcus-oscarsson
Copy link
Member Author

Yepp, we are using flake8, I should probably have been more specific with the noqa in this case is function complexity so we can disable that rule specifically.

@fabcor-maxiv
Copy link
Contributor

@marcus-oscarsson Yes, better with disabling only one specific rule, thanks :)

@marcus-oscarsson
Copy link
Member Author

marcus-oscarsson commented Oct 12, 2023

Sorry for the messy PR but it was a bit complicated to get things to run properly.

  • Files are now linted so that they pass flake8 (but without bug bear https://github.com/PyCQA/flake8-bugbear). We can enable it in another iteration.

  • test folder is not linted for the time being.

  • Formatted yaml and xml files (mostly white spaces) to pass pre-commit check-xml and check-yaml steps

  • Using micromamba and poetry for the installation, (we should probably update all the pipelines to use poetry for consistency).

@marcus-oscarsson marcus-oscarsson marked this pull request as ready for review October 12, 2023 19:56
@marcus-oscarsson marcus-oscarsson merged commit af160e4 into develop Oct 13, 2023
11 checks passed
@marcus-oscarsson marcus-oscarsson deleted the mo-lint branch October 13, 2023 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants