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

Fix: Replaced Black, isort, Flake8 with Ruff and Updated dependencies #580

Open
wants to merge 6 commits into
base: development
Choose a base branch
from

Conversation

alikhere
Copy link

@alikhere alikhere commented Mar 14, 2025

Description

This pull request simplifies the tooling in the project by replacing Black, isort, Flake8, and Pycodestyle with Ruff. The following changes have been made:

  • Removed the following unnecessary dependencies from the dev section of pyproject.toml as Ruff replaces them:

    • isort
    • flake8
    • pycodestyle
    • pyflakes
    • pep8-naming
  • Added Ruff as a dev dependency (ruff = "^0.11.0") to the pyproject.toml file. This consolidates both linting and formatting tasks under one tool.

  • Updated the pyproject.toml file to configure Ruff:

    • Added format settings to use spaces for indentation and enforce single quotes:
      [tool.ruff]
      format = { indent-style = "space", quote-style = "single" }
    • Updated the linting rules to include errors (E), warnings (W), flake-style issues (F), and ignored imports (I):
      lint = { select = ["E", "F", "W", "I"] }
    • Configured Ruff to recognize pretix as a known first-party package for sorting imports:
      isort = { known-first-party = ["pretix"] }
  • Updated GitHub Actions Workflow:

    • Replaced isort and flake8 jobs with a single Ruff job in .github/workflows/style.yml.
    • Added the following command to run Ruff for linting and formatting:
      ruff check --fix . && ruff format .
  • No Functional Changes:

    • This PR only updates code style and formatting. No functional changes have been made.

Fixes

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

The pull request #580 has too many files changed.

We can only review pull requests with up to 300 changed files, and this pull request has 523.

@alikhere alikhere changed the title docs: updated dependencies and replaced Black, isort, Flake8 with Ruff docs: replaced Black, isort, Flake8 with Ruff and updated dependencies Mar 14, 2025
@alikhere alikhere changed the title docs: replaced Black, isort, Flake8 with Ruff and updated dependencies fix: replaced Black, isort, Flake8 with Ruff and updated dependencies Mar 14, 2025
@hongquan
Copy link
Member

Sorry, we forgot to mention that single quote (') is preferred.

Copy link
Member

@hongquan hongquan left a comment

Choose a reason for hiding this comment

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

You need to update GH actions as well.

@hongquan
Copy link
Member

Hi @alikhere, the title of PR, please capitalize the beginning of sentence.
The common rule is that, any message for human to read, please apply grammar and spelling rule learned in school.

@alikhere alikhere changed the title fix: replaced Black, isort, Flake8 with Ruff and updated dependencies fix: Replaced Black, isort, Flake8 with Ruff and Updated dependencies Mar 15, 2025
@alikhere
Copy link
Author

ok

@alikhere alikhere changed the title fix: Replaced Black, isort, Flake8 with Ruff and Updated dependencies Fix: Replaced Black, isort, Flake8 with Ruff and Updated dependencies Mar 15, 2025
@alikhere alikhere requested a review from hongquan March 15, 2025 13:14
@alikhere
Copy link
Author

@hongquan Could you please review the PR.

@hongquan
Copy link
Member

@alikhere The purpose of "style" GH Action is to check if submitted code conform to standard, so it should not fix or write the code.

Your GH Action command is writing changes to the code:

ruff check --fix . && ruff format .

@hongquan
Copy link
Member

Did you test your changes in your localhost?

@alikhere
Copy link
Author

alikhere commented Mar 20, 2025

@alikhere The purpose of "style" GH Action is to check if submitted code conform to standard, so it should not fix or write the code.

Your GH Action command is writing changes to the code:

ruff check --fix . && ruff format .

sorry my bad, it should be

        ruff check .

@alikhere
Copy link
Author

Did you test your changes in your localhost?

I tried to setup project locally using Dockerfile but getting error while buiding docker image.

Screenshot from 2025-03-20 05-56-12

@hongquan
Copy link
Member

hongquan commented Mar 20, 2025

You just need to test the lint tool, you don't need to run the application, don't need to setup Docker.
Btw, please increase line length to 120.

@alikhere
Copy link
Author

You just need to test the lint tool, you don't need to run the application, don't need to setup Docker. Btw, please increase line length to 120.

I have tested the linting tool by running ruff check . and verified that it detects linting issues based on the selected rules (E, F, W, I)

image

@hongquan
Copy link
Member

Please configure Ruff to ignore F405 error in settings.py files.
It would be nice if you can read those Ruff report and decide which errors can be ignored, which errors should be fixed later.

@alikhere
Copy link
Author

Please configure Ruff to ignore F405 error in settings.py files. It would be nice if you can read those Ruff report and decide which errors can be ignored, which errors should be fixed later.

I have tested the changes, and now ruff successfully ignores F405 in the relevant settings.py files.

@hongquan
Copy link
Member

Every time you make some changes, you should show the report so that I can see and determine what should do next.
How about this idea:

It would be nice if you can read those Ruff report and decide which errors can be ignored, which errors should be fixed later.

Can you do or not?

@alikhere
Copy link
Author

Every time you make some changes, you should show the report so that I can see and determine what should do next. How about this idea:

It would be nice if you can read those Ruff report and decide which errors can be ignored, which errors should be fixed later.

Can you do or not?

Good idea, i can do that.

@alikhere
Copy link
Author

alikhere commented Mar 23, 2025

Please configure Ruff to ignore F405 error in settings.py files. It would be nice if you can read those Ruff report and decide which errors can be ignored, which errors should be fixed later.

Report

1. Resolved Merge Conflicts

  • New code is added to upstream development branch which caused conflicts with my branch so i fetch the latest changes and resolved the conflicts.

2. Ignored F405 in settings.py

Before ruff was reporting F405 warnings in settings.py.

Screenshot from 2025-03-21 16-49-24

After that i added the following rule to pyproject.toml

[tool.ruff.lint.per-file-ignores]
"*/settings.py" = ["F405"]

then ran ruff check . and verified that Ruff no longer reports F405 in settings.py.

Screenshot from 2025-03-21 16-50-21

3. Applied Ruff Formatting on Merged Files

Since merge conflicts introduced new code, I ran ruff format .
This applied Ruff's formatting rules (line length 120, single quotes, etc.) to the updated files.

All changes have been tested, and Ruff is now correctly ignoring F405 in settings.py.

@hongquan
Copy link
Member

hongquan commented Mar 26, 2025

@alikhere I wonder if you are just repeating ChatGPT answer or you did put some personal thought to your work. The last comment #580 (comment) is not related to what I suggested you to do. It is just a repetition of #580 (comment).

@alikhere
Copy link
Author

@alikhere I wonder if you are just repeating ChatGPT answer or you did put some personal thought to your work. The last comment #580 (comment) is not related to what I suggested you to do. It is just a repetition of #580 (comment).

Apologies for the misunderstanding. I initially thought you wanted a summary of the changes I made, rather than an analysis of the ruff report itself. I will now go through the report carefully identify which issues should be ignored or fixed.

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