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

chore/fix: re-enable linting for backend #2041

Open
wants to merge 8 commits into
base: dev
Choose a base branch
from

Conversation

akx
Copy link
Contributor

@akx akx commented May 7, 2024

Description

This PR re-enables linting for the backend (disabled 4 months ago in #368 because "too many issues") using Ruff, and fixes the issues it finds. Each commit should be fine to review in isolation, if this PR otherwise feels too large. Most of the cumulative diff is reordering imports.

I deliberately disabled some rules for the time being, though, with TODO notes that they really ought to be fixed; bare try:except:s for one are a bad habit, since they catch e.g. KeyboardInterrupts, so chances are the backend will ignore attempts of stopping it if the signal lands in a suitable place 😅
Conversely, more rules could be added later.

It also switches formatting from black to ruff (they're 99.x% compatible, but ruff is a lot faster and formats some things in a neater way). The formatting actually found a couple of stray commas that inadvertently turned some logging statements into no-op 1-tuple constructions... 😄

For CI, I made sure that the pinned versions from requirements.txt are used for Ruff – before this, CI could install a different version of Black than what requirements.txt said.

Testing

Since there is no test suite, all I really could do is test the app runs fine with these changes.

I ran under coverage (coverage run -m uvicorn main:app ...), which gave me a coverage of about 49%.

Dependencies

black was dropped in favor of ruff; technically, Ruff should be a development-time dependency only, but there is no requirements-dev.txt or similar and Black was in the runtime deps, so I didn't change that.


Changelog Entry

Changed

  • Python code is now linted and formatted using Ruff.

@akx akx force-pushed the lint-backend branch 4 times, most recently from 2398d65 to 3fddd6c Compare May 8, 2024 09:41
@akx
Copy link
Contributor Author

akx commented May 13, 2024

@cheahjs Please let me know if there's any interest in getting this merged (I see there's adjacent work in #2182) so I know whether or not to keep rebasing this.

@akx
Copy link
Contributor Author

akx commented May 14, 2024

Rebased on dev. cc @cheahjs @tjbck

@akx
Copy link
Contributor Author

akx commented May 27, 2024

@tjbck Rebased once more on dev.

@justinh-rahb
Copy link
Collaborator

@tjbck have we reached a decision on this?

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.

None yet

2 participants