-
Notifications
You must be signed in to change notification settings - Fork 7
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
Initial setup for pre-commit and ruff (copied from GRASS GIS repo) #45
base: main
Are you sure you want to change the base?
Conversation
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
use docker v2 cmd
I am currently preparing a PR for STRDS support. Would be great if this could be merged rather soon, and then #44 as the following PR wil build upon that. Then formating things can be addressed in subsequent PRs.... |
BTW: de0e435 fixes the failing integration tests because until then, docker-compose v1 was still in use... |
In general this is very cool and enhances the quality a lot! Thanks for this! I am hesitating a bit to approve because we are preparing to make the post-pr feature (also copied from GRASS GIS repo) with black and ruff usable via the shared github-workflows we use a lot to not have to copy & paste all config files to all repositories. It is also planned to add the linting rules selection used by GRASS GIS to the config which could then be shared via And also I am not so sure about the line length - mostly we try to be very strict and stick to the 79 characters per line to have it eqal in the different repositories. But I am open for discussion regarding this one 🙃 Please share your thoughts on the shared workflows and if you would be willing to use them. It might be more easy to first add all these cool enhancements here and then move them to the shared repo step by step, as the basic linting workflows are also not reused yet in this repository and would need to be replaced anyway. |
Hi @mmacata shared workflows sound like a smart solution to me. I was not aware of that feature, so I am for sure willing to use them and it is perfectly fine for me to close this PR in favour of shared workflows. I have no opinions about the line length. Introduction of 88 charcters length would lead to re-formating of a lot of the code... I thought 88 characters was a kind of standard for black, but I would have no issues with 79. As for introduction of formating: compliance with ruff all rules would have to be implemented in several steps anyway I guess. There are many required changes if all rules are supposed to be applied and only a low-double-digit percentage could be fixed automatically... Thus I think either way, ruff would have to be introduced through several PRs... You could either start implementing ruff compliance here and then replace it with the standardized/shared solution once that is ready, or you make repo-specific exceptions to the shared solution and fix them one by one later on... That said, either one is fine with me. I have as mentioned no problem with replacing this PR with a shared solution... You decide. If you go for the shared workflows first, we could try to get #44 in first, so I can use the standardized |
This PR sets up configuration for pre-commit and ruff plus github actions in a way that does not require changes in the code itself. Adjustments for code formating can be done in subsequent PRs where e.g. single ruff rules are addressed (or newer versions of black formating)...