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

Set up pre-commit hooks to enhance code formatting and linting #196

Merged
merged 17 commits into from
Apr 10, 2024

Conversation

ben9809
Copy link
Contributor

@ben9809 ben9809 commented Mar 11, 2024

This pull request introduces pre-commit hooks to our project to automate and enhance code quality. Integrating these hooks will ensure that every commit complies with our coding standards.

Changes include:

  • Added a .pre-commit-config.yaml file configuring Black and Flake8.
  • Configured Black for consistent code formatting.
  • Configured Flake8 for linting, helping identify and prevent potential code quality issues.

Developers must set up pre-commit on their local environments to benefit from these hooks.

@ben9809 ben9809 requested a review from jasonlyik March 11, 2024 21:38
@jasonlyik
Copy link
Contributor

@ben9809 Looks pretty good.
I tried setting up a test file and I noticed the following:

  • The file is skipped by pre-commit unless it is tracked by git add
  • After automatic reformatting, I will need to git add again in order to track the format changes.

These are both expected behavior, right? I think we can add a bit more detail to the contributing text if so.

@ben9809
Copy link
Contributor Author

ben9809 commented Mar 15, 2024

Yes, the behavior you're observing is expected when working with pre-commit hooks in git.
It's a good idea to clarify these points in the contributing guidelines, I'll take care of it!

@ben9809
Copy link
Contributor Author

ben9809 commented Mar 15, 2024

I'd like to propose the following enhancement for our code quality process:

we could enforce our code quality standards server-side, by integrating pre-commit checks into our GitHub Actions workflow, such that if a developer forgets to run pre-commit locally, our codebase's integrity is safeguarded, as the checks will automatically run on the server when a pull request is opened on the dev branch. (This is still not integrated into this pull request)

Let me know your thoughts @jasonlyik!

@jasonlyik
Copy link
Contributor

Sounds great, I think doing the server-side checks and making the format changes at PR time sounds pretty good, so that it's easier to develop locally.

@ben9809
Copy link
Contributor Author

ben9809 commented Mar 18, 2024

@jasonlyik Before we merge this pull request, I'd like to discuss some issues with certain files during our next development meeting. This way, we can decide together how to manage them!

@jasonlyik
Copy link
Contributor

Sounds good, let's take a look next meeting

@jasonlyik jasonlyik merged commit 80d71e9 into dev Apr 10, 2024
1 check passed
@jasonlyik jasonlyik deleted the pre-commit-config branch April 12, 2024 14:21
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