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

NeuroBench / neurobench

Benchmark harness and baseline results for the NeuroBench algorithm track.

https://neurobench.readthedocs.io

Apache License 2.0

55 stars 12 forks source link

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

Closed ben9809 closed 5 months ago

ben9809 commented 6 months ago

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.

jasonlyik commented 6 months ago

@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 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

@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 commented 6 months ago

Sounds good, let's take a look next meeting