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

add pre-commit and ruff as dev dependencies #43

Closed
haleemur opened this issue Aug 28, 2024 · 0 comments · Fixed by #47
Closed

add pre-commit and ruff as dev dependencies #43

haleemur opened this issue Aug 28, 2024 · 0 comments · Fixed by #47
Labels
automation Release and test automation good first issue Good for newcomers

Comments

@haleemur
Copy link
Contributor

Description

I think we should add pre-commit and ruff as dev dependencies, and update the readme to instruct contributors to ensure pre-commit is set up when they clone this project.

This would help ensure pull requests are less likely to fail a check in the CI.

The current linting uses a combination of black, flake8, isort, pydocstyle - all of these can be replaced by ruff.

@edgarrmondragon edgarrmondragon added automation Release and test automation good first issue Good for newcomers labels Aug 28, 2024
edgarrmondragon added a commit that referenced this issue Sep 23, 2024
# Description

* Replaces black, flake8, pydocstyle & isort with Ruff
* Adds `.pre-commit-config.yaml` file
* Fix some linting issues highlighted by Ruff in default setting, while
disabling a 5 rules. The following rules are disabled:
    - ANN001
    - ANN401
    - S608
    - SLF001
    - EM101
    
`ANN001` * `ANN401` are disabled because the codebase has a few
instances where params / return values don't specify type or return
`Any`.

`S608` flags string based queries. It will require a deeper refactor to
leverage sqlalchemy exclusively.

`SFL001` flags accesses to private members. This is done to patch
`_confirm_primitive_property`.

`EM101` flags strings passed to Exceptions, which I felt was safe to do.

closes: #43

---------

Co-authored-by: Edgar Ramírez Mondragón <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automation Release and test automation good first issue Good for newcomers
Projects
Development

Successfully merging a pull request may close this issue.

2 participants