-
Notifications
You must be signed in to change notification settings - Fork 23
fix charm4py formatting and add check #305
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
base: main
Are you sure you want to change the base?
Conversation
I think adding a linter to CI is great. A couple of suggestions:
|
I moved the CI test into the main CI file. However, i tried using ruff on my local machine and I think it's too aggressive; it checks for things like undefined variables/star imports/other problems that I am worried will cause the code to break. I did apply some of the suggested ruff fixes here but I think the CI should just check for formatting. For further context |
We should probably discuss in the meeting whether it will cause any issues tracking changes with git blame. But otherwise seems like a good idea to use a linter
Hmm, that is confusing, since https://github.com/super-linter/super-linter appears to include ruff as well? Can you disable the errors that you think are too aggressive globally in pyproject.toml (see e.g. https://github.com/inducer/meshmode/blob/89fcb4955b160098e23389801454b9e90fb91e25/pyproject.toml#L74 for an example). My concern about using a github-actions specific formatting check is that it makes it harder to reproduce error checks locally. |
I already have done that, it only checks for Black-based formatting. The way the super linter works is that there a bunch of "VALIDATE_" environment variables that check for things, and if you set one to TRUE (like setting VALIDATE_PYTHON_BLACK to true), it will turn off all the other checks. You can also see in the actions that only the Black check is being run. |
Could you just run |
Ruff format is a bit different than black, and checks for both will have different results. Which one is best? |
Hmm, they should be almost the same (https://docs.astral.sh/ruff/faq/). I would go with ruff format, so we can easily "graduate" to do additional checks in the future. |
This fixes all formatting in the codebase to PEP 8 standards, and uses the super-linter repo to enforce the formatting in Github Actions. The Black tool can be used to fix any formatting issues.