Added missing .pre-commit-config.yaml

[marcus-oscarsson]

This is a basic .pre-commit-config.yaml, the configuration in pyproject.toml was not sufficient to run pre-commit. I guess its been broken somewhere along the road since we have not tested it for some time.

We can now start to use and build on this.

I've disabled flake8 and some other checks for the moment the first because there are simply quite alot to deal with at the moment but we can eventually enable it.

To install and run this locally so that is automatically executed on commit:

# Install the tool
pip install pre-commit

# Install the hook in your .git folder
pre-commit install

#run it manual once
pre-commit run

There are some more instructions on how to run/setup this locally here:

https://pre-commit.com/#install

This builds on what @oldfielj-ansto (thanks by the way :) ) started, anyone feel free to elaborate :)

[elmjag]

Great initiative!

A couple of questions...

Are pre-commit checks supposed to be enabled in the pre-commit.yml workflow? For example, like here: https://github.com/elmjag/mxcubecore/commit/d9911cd5645899e5c71a60efe4a47e057c845dcb

Also, the pre-commit checks are failing, as far as I can tell:

https://github.com/elmjag/mxcubecore/actions/runs/6145710975/job/16673614950#step:6:32

Is the plan for merge this PR first, and then fix code to pass pre-commit checks? Or the other way around?

Is there any plans for fixing failing checks? I can help if needed!

[marcus-oscarsson]

The pre-commit hook checks that the source is up to standards before committing. So you need to install this routine/hook locally so that git will pick it up and run it for you.

Fail or pass will depend on your changes, when I run pre-commit without any changes I get:

/mxcubecore$ pre-commit
poetry-check.........................................(no files to check)Skipped
Autoflake............................................(no files to check)Skipped
Black................................................(no files to check)Skipped
trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check for case conflicts.................................................Passed
check for merge conflicts................................................Passed
check for broken symlinks............................(no files to check)Skipped
check xml............................................(no files to check)Skipped
check yaml...........................................(no files to check)Skipped
mixed line ending........................................................Passed

The pre-commit CI runs the same thing and makes sure that the pre-commit hook was executed/respected, if not it will fail.

[marcus-oscarsson]

In your case: https://github.com/elmjag/mxcubecore/actions/runs/6145710975/job/16673614950#step:6:32 it seems like both Autoflake and Black fails meaning that your source fails are not correctly formatted. Running Autoflake or Black on the files you CHANGED files should do the trick.

Actually looking closer the CI pipeline job has --all-files switch witch checks all files which assumes that the state before your commit passes, which is currently not our case, thats why we for the moment have disabled the CI job.

[elmjag]

I understand the logic with pre-commit hook is only applied to modified files on commit.

However, it's gonna be somewhat troublesome to use if not all files in repository can pass the pre-commit checks. As soon as you touch a file that is not passing, the pre-commit check will fail for you.

Also, not every commiter will have pre-commit hooks installed, thus we'll get new changes that are not passing the checks.

IMHO 'pre-commit run --all-files' check in CI makes a lot of sense. In the long run, it will make working with the code easier.

But OK, I understand that enabling pre-commit in CI is outside of the scope for this PR. Let's merge this first. We can discuss how to enable '--all-files' check in the future.

[marcus-oscarsson]

Exactly, so thats why until now we disabled the pre-commit hook until we made the effort to bring all the file to the same standards so to speak.