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

Lint and format code in CI and pre-commit #3085

Merged
merged 30 commits into from
Jul 22, 2024
Merged

Conversation

bolinocroustibat
Copy link
Contributor

@bolinocroustibat bolinocroustibat commented Jul 10, 2024

Remark 1:
Since this PR is heavy, I suggest checking the changes/diff before the commit 6d1b9aa ("style: lint and format code and sort imports"), this commit being the one dedicated to lint and format the code and making the whole PR barely readable :)

Remark 2:
I noticed the team might prefer single quotes (') over double quotes ("). Ruff/Flake8 default rule is to use double quotes, but we can configure a special rule for to use single quotes if the team prefers.
In that case, we can either use a ruff.toml to specify this special formatting rule:

[format]
quote-style = "single"

...or a pyproject.toml config file if we want to use a general Python package config file (so the config file can be used for many other config, and/or for Python packaging):

[tool.ruff.format]
quote-style = "single"

Let me know if you want to add those special styles and/or any other, I can modify the code format accordingly in this same PR.

@bolinocroustibat bolinocroustibat requested review from maudetes and ThibaudDauce and removed request for maudetes and ThibaudDauce July 10, 2024 15:41
@bolinocroustibat bolinocroustibat force-pushed the lint-and-format branch 2 times, most recently from 5a60de6 to 6d48b26 Compare July 10, 2024 16:15
@bolinocroustibat bolinocroustibat removed the request for review from maudetes July 10, 2024 16:41
@bolinocroustibat bolinocroustibat marked this pull request as draft July 10, 2024 16:41
@bolinocroustibat bolinocroustibat requested review from maudetes and ThibaudDauce and removed request for maudetes July 10, 2024 16:56
@bolinocroustibat bolinocroustibat marked this pull request as ready for review July 10, 2024 16:57
@bolinocroustibat bolinocroustibat force-pushed the lint-and-format branch 2 times, most recently from 1018369 to 6987454 Compare July 11, 2024 12:56
@bolinocroustibat bolinocroustibat changed the title Lint and format Lint and format code in CI and pre-commit Jul 12, 2024
Copy link
Contributor

@maudetes maudetes left a comment

Choose a reason for hiding this comment

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

Thank you! I think this is much needed :) Ruff seems indeed quite efficient.

Some general comments :

  • flake8 config in setup.cfg should be removed?
  • I think keeping single quotes would allow a lesser impact on git history
    • I would go with the pyproject approach for the config
  • on a second iteration, we can add this config on all udata plugins I think 🚀

.circleci/config.yml Outdated Show resolved Hide resolved
.pre-commit-config.yaml Show resolved Hide resolved
Comment on lines 45 to 46
ruff check --fix --select I .
ruff format .
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure I see the results of each command separately.

@bolinocroustibat bolinocroustibat merged commit dc90df5 into master Jul 22, 2024
1 check passed
@bolinocroustibat bolinocroustibat deleted the lint-and-format branch July 22, 2024 08:27
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