florianschanda / miss_hit

MATLAB Independent, Small & Safe, High Integrity Tools - code formatter and more
GNU General Public License v3.0
160 stars 21 forks source link

overzealous miss_hit in pre-commit CI #216

Closed Remi-Gau closed 3 years ago

Remi-Gau commented 3 years ago

Hey,

Not sure this is purely miss_hit related but reporting it here in case someone has an idea.

I am using miss_hit on one of my repo with precommit.

I wanted to try the pre-commit CI as it can run mh_style --fix automatically and update an open PR with those fixes (not necessary if you are using precommit locally, but nice to have in case colleagues or contributors don't run miss_hit).

More info here: https://pre-commit.ci/

So it is easy to set up and will run on your PR no problem (and update it): Example: https://github.com/cpp-lln-lab/CPP_SPM/pull/378

The problem seems to be that in CI miss_hit is not ignoring the folders that it should ignore: See this report where the lib folder is being "searched" by miss_hit eventhough the config tells it not to.

https://results.pre-commit.ci/run/github/208231636/1623701596.tnPwryOERc2qQZ_-Qsg64w

This is not a problem that urgently needs a solution but in case someone else comes across the issue or has an idea on how to solve this.

Pinigng @wbekerom as he was involved in the pre-commit PR #185

florianschanda commented 3 years ago

Is it possible to reproduce locally (i.e. running mh_style on a specific revision on that repo will trigger changes in lib)? If so can you send me a revision I could try this on?

florianschanda commented 3 years ago

Right I think I know what the problem is; in that if you provide a file to mh_style in an excluded dir, it is being checked. I.e.

mh_style . (will exclude all things in lib/) mh_style lib/foo.m (will analyze foo.m)

The reason is that exclude_dir is meant for excluding external repositories; which themselves may or may not have a miss_hit config.

You could do:

florianschanda commented 3 years ago

So I guess that https://github.com/cpp-lln-lab/CPP_SPM/blob/474d4e6f7aaf8b1f50f3ba4254e6114d831ed89d/.pre-commit-config.yaml#L11 will cause the CI tool to give all the files, instead of running on root

In which case only the enable: false will work for you i think

Remi-Gau commented 3 years ago

The enable:false worked: pre-commit-pr is now quiet and stays out of my lib folder.

https://github.com/cpp-lln-lab/CPP_SPM/pull/383

Thanks for the tip.

Will close this.

florianschanda commented 3 years ago

Glad I could resolve the issue. As always it's really nice to see people using this in anger ;)