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

Ruff everything #54

Merged
merged 1 commit into from
Dec 16, 2024
Merged

Ruff everything #54

merged 1 commit into from
Dec 16, 2024

Conversation

msaroufim
Copy link
Member

@msaroufim msaroufim commented Dec 16, 2024

Description

This PR is a 1 time lint job so the repo is now in a state where the code style can stay consistent. I'm doing it now because the more we delay it the more annoying things become. Let's please merge this asap, otherwise I'll have to relint

  1. So we introduce a new job called ``lint.yml` which fails if code is not linted
  2. Locally developers should ruff check . to see which failures they need to fix
  3. We can add or remove new linting rules in .ruff.toml

Some important fixes

  1. we were returning in finally blocks which would swallow all errors so rewrote those away
  2. We had some long lines I rewrote or broke apart. The max line limit is still too long and it's worth trying to get it below 100 for a future PR
  3. I deleted a testing file called github_only which we're not really using anymore

Checklist

  1. I ran run modal which worked https://discord.com/channels/1303055581078093874/1305687036723859587/1318054635969183756
  2. I ran run github which worked https://discord.com/channels/1303055581078093874/1318057422367100938/1318057425546383381

@msaroufim msaroufim merged commit 47d621b into main Dec 16, 2024
1 of 3 checks passed
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