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-hooks #2128

Merged
merged 3 commits into from
Jun 12, 2024
Merged

Add pre-commit-hooks #2128

merged 3 commits into from
Jun 12, 2024

Conversation

k9845
Copy link
Contributor

@k9845 k9845 commented May 2, 2024

Addresses

TODO For future PRs

Changes

  • Introduce pre-commit tool to run locally and through CI
    • black
    • isort

WARNING

To enforce this checks we need to update the current code base using black and isort which will change almost all the codebase. We will need to do that changes after merging all the pending PRs to avoid conflicts.

@susilnem susilnem requested a review from thenav56 May 3, 2024 06:12
@thenav56
Copy link
Member

thenav56 commented May 5, 2024

We will also need to update our pyproject.toml.
NOTE: Make sure extend-exclude/skip has same behavior as .pre-commit exclude

[tool.black]
line-length = 130
# NOTE: Update in .pre-commit-config.yaml as well
extend-exclude = "(__pycache__|.*snap_test_.*\\.py|.+\\/.+\\/migrations\\/.*)"

[tool.isort]
profile = "black"
multi_line_output = 3
# NOTE: Update in .pre-commit-config.yaml as well
skip = [
    "**/__pycache__",
    "**/snap_test_*.py",
    ".venv/",
    "legacy/",
    "**/migrations/*.py",
]

@k9845 k9845 force-pushed the feature/pre-commit-hooks branch from 2221f30 to 97bfc1c Compare June 3, 2024 10:09
@k9845 k9845 marked this pull request as ready for review June 3, 2024 10:09
@k9845 k9845 force-pushed the feature/pre-commit-hooks branch from 7b6f322 to b48c1ef Compare June 4, 2024 04:45
@thenav56 thenav56 force-pushed the feature/pre-commit-hooks branch 2 times, most recently from 8f5ad00 to ebdad0f Compare June 11, 2024 10:19
Copy link
Contributor Author

@k9845 k9845 left a comment

Choose a reason for hiding this comment

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

LGTM

@szabozoltan69 szabozoltan69 merged commit 576f03c into develop Jun 12, 2024
2 checks passed
@szabozoltan69 szabozoltan69 deleted the feature/pre-commit-hooks branch June 12, 2024 08:39
@szabozoltan69
Copy link
Contributor

Deployed to Staging and even to Prod also (with an other fix).

@thenav56 thenav56 mentioned this pull request Jun 27, 2024
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.

3 participants