Skip to content

Conversation

@ReenigneArcher
Copy link
Member

Double check these details before you open a PR

  • PR does not match another non-stale PR currently opened

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 requests is extremely stable as far as not introducing breaking changes, so I believe this is safe to pin to v2.*.

@Snailedlt
Copy link
Collaborator

Linting is definitely useful for our projects!
Personally I prefer the Ruff linter for Python projects, since it's very similar to Black, but also has formatting built in. It's basically prettier for python :)

Any reason why you used flake8 over Ruff or Black here?

@ReenigneArcher
Copy link
Member Author

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.

@Snailedlt
Copy link
Collaborator

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants