search

macformula / racecar

Monorepo for all firmware running on our custom ECU's
6 stars 1 forks source link

pre-commit actions #76

Open BlakeFreer opened 5 months ago

BlakeFreer commented 5 months ago

Create a pre-commit file to allow users to install git hooks locally. See https://pre-commit.com/

Any check that is performed by a github action should be included as a hook to allow the coder to detect simple issues before pushing. For example, there should be a pre-push hook to run the same clang-format check as performed in GitHub actions.

Another good hook would be pre-commit to prevent committing directly to the main branch.

It is important to understand that these hooks do not prevent the user from doing anything (they could easily not install the hooks), but help the user avoid errors that would be generated when then try to make a PR.

Deliverables:

  1. .pre-commit-config.yaml file with at least the 2 hooks mentioned here. The file should be located in racecar/firmware racecar
  2. An updated firmware/readme Edit the developer setup guide in docs/docs/firmware/dev-setup.md to specify the MINIMUM python version required to install pre-commit
BlakeFreer commented 2 weeks ago

We could have a clang-format (or better, git-clang-format) before any commit. See https://pypi.org/project/clang-format/

Micnasr commented 2 weeks ago

Is that not what we already have? I remember implementing a git clang format after every PR to not allow any code that did not follow the formatting rules.

BlakeFreer commented 2 weeks ago

Currently we have a GitHub pull request action that rejects a PR if the code is not clang-formatted.

This suggestion would execute git-clang-format locally whenever the developer makes a local commit. This ensures that all committed code is formatted before becoming a PR, which saves the developer time since they don't need the PR action to tell them to fix the code.

See https://www.reddit.com/r/Python/comments/17gnspo/when_to_use_precommit_hook_vs_github_actions/

Micnasr commented 1 week ago

the .pre-commit-config.yaml has to be placed in the root of the directory from my understanding. The configuration works for me locally when i run: pre-commit run --all-files but it does not detect the file if its not in the root when commiting. What should I do?

BlakeFreer commented 1 week ago

I think you should remove the files= line from the clang-format configuration. Use git-clang-format instead and let it figure out which files to change