apl-ocean-engineering / sonar_image_proc

Other
12 stars 3 forks source link

MWE of using pre-commit to manage pre-commit checks #28

Closed amarburg closed 1 year ago

amarburg commented 1 year ago

A first-cut MWE of using pre-commit to manage Git pre-commit hooks.

To "activate" pre-commit on your checkout, you need to:

  1. Install the Python package e.g. pip install pre-commit
  2. In your checkout, pre-commit install

To manually check all files: pre-commit run --all-files, otherwise the checks will run (duh) pre-commit.

Note: that many of the checks (e.g. clang) will update your files automatically. It is up to you to commit the resulting changes.


The config file included here runs a number basic file heuristics (whitespace at the end of files, etc), and clang-format with the "Google" style.

cpplint is included but commented out (using it will require the time commitment to fix the resulting warnings).

For reference, this is the output when I run it manually:

Untitled

amarburg commented 1 year ago

As of right now, running pre-commit does not run in CI. It needs to be:

  1. Added to the relevant CI images
  2. Explicitly called from the CI script

and ideally

  1. The CI cache should be configured to save pre-commit's checkouts of the necessary tools (see the pre-commit manual)
amarburg commented 1 year ago

Just for simplicity, I put the messy commit containing all of the minor changes from clang-format and other hooks in its own PR

amarburg commented 1 year ago

@micat001 @lauralindzey per last-week's conversation I decided to check out pre-commit to manage auto-magic pre-commit linting and formatting. I'm reasonably impressed.

At your leisure, try this branch. It does not have the suggested changes committed (see #29 ), so you can use pre-commit run --all-files to see before-and-after.

Concerns:

amarburg commented 1 year ago

Ha ha. I modified our ROS-CI process to check for a .pre-commit-config.yaml file.... and it works! But this branch doesn't pass the CI check (because it doesn't contain the "fixes" from pre-commit). Sadness.

lauralindzey commented 1 year ago

This is officially really cool. I'm sorry it took me so long to get around to looking at this PR. I was just telling @micat001 this morning that I wished we had auto-linting set up .... and here you've started down that path!