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

Switch to Ruff #1005

Merged
merged 2 commits into from
Oct 31, 2024
Merged

Switch to Ruff #1005

merged 2 commits into from
Oct 31, 2024

Conversation

tuomas777
Copy link
Contributor

@tuomas777 tuomas777 commented Oct 18, 2024

This PR introduces Ruff as the new linter and formatter, replacing the previously used tools: Black, Flake8, and isort. The goal is also to standardize the linting and formatting rules across all of our backend projects.

One key change to linting/formatting rules is the reduction of the linter maximum line length from 120 to 88 characters. Consequently, hundreds of lines exceeding the new limit are marked with # noqa: E501 to prevent linting errors. These comments can be removed gradually as the codebase is updated to comply with the new standard.

The PR includes a significant number of modified lines, primarily due to automatic formatting by Ruff. Reviewers can safely skip the following commits during manual review, as they involve safe automatic formatting:

Some automatic changes made by Ruff and autopep8 (which was run to help fix excessively long lines) may require some manual inspection, as they could be potentially unsafe:

After the review and before merging, the plan is to squash commits for a cleaner history, perhaps combining the current chore commits into one and the style commits into another (which can then be added to .git-blame-ignore-revs).

Update: Added some new rules to Ruff lint config:

  • flake8-bugbear (excluding opinionated rules 9xx) and pep8-naming: these are in use with Flake8 in many of our projects, so let's keep using them
  • flake8-print and flake8-pie: these are pretty trivial to add

New commits start from cc32264

Refs LINK-2201

@tuomas777 tuomas777 force-pushed the switch-to-ruff branch 2 times, most recently from 87ea40e to 431b4f0 Compare October 18, 2024 12:42
@terovirtanen
Copy link
Contributor

LINKEDEVENTS-API branch is deployed to platta: https://linkedevents-pr1005.api.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

LINKEDEVENTS-API branch is deployed to platta: https://linkedevents-pr1005.api.dev.hel.ninja 🚀🚀🚀

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 95.23810% with 12 lines in your changes missing coverage. Please review.

Project coverage is 91.32%. Comparing base (ea21c23) to head (90f934d).

Files with missing lines Patch % Lines
events/schema.py 91.46% 7 Missing ⚠️
events/importer/enkora.py 0.00% 2 Missing ⚠️
events/exporter/city_sdk.py 0.00% 1 Missing ⚠️
...vents/management/commands/add_helsinki_audience.py 0.00% 1 Missing ⚠️
.../management/commands/populate_local_event_cache.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1005      +/-   ##
==========================================
+ Coverage   91.29%   91.32%   +0.02%     
==========================================
  Files         405      405              
  Lines       38730    38711      -19     
==========================================
- Hits        35357    35351       -6     
+ Misses       3373     3360      -13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tuomas777 tuomas777 force-pushed the switch-to-ruff branch 2 times, most recently from 5a01f6c to 3b487c9 Compare October 23, 2024 20:26
@terovirtanen
Copy link
Contributor

LINKEDEVENTS-API branch is deployed to platta: https://linkedevents-pr1005.api.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

LINKEDEVENTS-API branch is deployed to platta: https://linkedevents-pr1005.api.dev.hel.ninja 🚀🚀🚀

@tuomas777 tuomas777 changed the title WIP Switch to ruff Switch to Ruff Oct 24, 2024
@tuomas777 tuomas777 marked this pull request as ready for review October 24, 2024 15:34
@tuomas777 tuomas777 requested a review from a team October 25, 2024 07:50
Copy link
Contributor

@danipran danipran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, left a question about mccabe

requirements-dev.txt Show resolved Hide resolved
@tuomas777 tuomas777 force-pushed the switch-to-ruff branch 2 times, most recently from a71cec5 to 6bcc31a Compare October 28, 2024 23:04
@terovirtanen
Copy link
Contributor

LINKEDEVENTS-API branch is deployed to platta: https://linkedevents-pr1005.api.dev.hel.ninja 🚀🚀🚀

Copy link
Contributor

@danipran danipran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 👍

@terovirtanen
Copy link
Contributor

LINKEDEVENTS-API branch is deployed to platta: https://linkedevents-pr1005.api.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

LINKEDEVENTS-API branch is deployed to platta: https://linkedevents-pr1005.api.dev.hel.ninja 🚀🚀🚀

Copy link

sonarcloud bot commented Oct 30, 2024

@terovirtanen
Copy link
Contributor

LINKEDEVENTS-API branch is deployed to platta: https://linkedevents-pr1005.api.dev.hel.ninja 🚀🚀🚀

@tuomas777 tuomas777 merged commit c422936 into main Oct 31, 2024
19 of 21 checks passed
@tuomas777 tuomas777 deleted the switch-to-ruff branch October 31, 2024 07:41
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.

4 participants