ci(lint): add linting workflow #2362
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Linting is definitely useful for our projects! Any reason why you used flake8 over Ruff or Black here? |
|
I just wanted to start with flake8 because I believe it's less strict than black. I'm open to using whichever one in the end though. |
|
Sounds good to me, let's start with flake8, we can always switch to an alternative later on if needed |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Double check these details before you open a PR
Features
This PR introduces linting for python files, yaml files, and docker files. This file is borrowed from my @LizardByte org, and modified. We use this file in every repo in our org, so it is very well tested.
While going through #2295 and other PRs, I've discovered a lot of common linting issues. Linting the python files mostly just helps with code standardization, but can also find some unpredictable bugs (like mixing and matching tabs/spaces). TODO: address lint errors/warnings before merging this.
Linting yaml files can also help avoid annoying whitespace related bugs and catch these kind of problems before they ever start. TODO: address lint errors/warnings before merging this.
Dockerfiles are linted using hadolint, which does both dockerfile linting as well as shellcheck linting. I have updated the dockerfile to pass the hadolint errors, which were to use a specific tag on the image (gitpod also suggests this in their docs), clean apt cache, and not use cache for pip installation. Additionally, hadolint complains about using
sudo; however I have ignored that warning as I think it's required in the gitpod image. I have some standard ignored errors in the workflow file as well so these don't need to be specifically ignored.This PR closes NONE
Notes
I will fix the python and yaml lint errors after #2295 is merged (to avoid a rebase nightmare).
I also updated requests in the dockerfile to use the latest version as long as it's under version 3. The previously specified version has security vulnerabilities. From my experience over the years
requestsis extremely stable as far as not introducing breaking changes, so I believe this is safe to pin tov2.*.