Remove the `pre-commit.ci` PR check

[simlmx]

We should remove it as it is redundant where the lint checks defined in the repo.

I am not sure where it is defined. There is a [.pre-commit-config.yaml](https://github.com/openclimatefix/PVConsumer/blob/main/.pre-commit-config.yaml) at the root of the repo but removing it is not enough.

CC @peterdudfield

[jacobbieker]

Its partly defined in the organization. The point of it is that is also fixes some linting problems, primarily black formatting automatically, if someone isn't using the pre-commit hooks.

[simlmx]

I think we should revisit the idea of having lint rules defined in at the organization level. My opinion is that guidelines would be better, and let each repo define its own lint rules. The reason is that it makes the repo self-contained and we can format the code from within it, instead of making a PR and waiting for the CI to do it.

Currently in the repo you can run make format (an alias to run black, isort and ruff). make lint will check that nothing needs to be done for the same scripts and I made the CI run that instead of the organization github workflow.

The problem with the organization-level workflows is that they are hard to reproduce locally. You need to find where they are defined, then install the dependencies (which will get outdated) by hand somewhere, etc. This is just too much trouble compared to having something defined inside the repo IMO.

[jacobbieker]

I think having them defined organization level means that there is then consistency across our repos on how our code is linted/looks/what's expected for it that is then defined in one place for all the repos. Its then one configuration for all of OCF's code. And with pre-commit you can easily install and run it locally in a pre-commit hook if desired for our repos, which is the exact same as our organization-level rules, or let the GH Action do some of the formatting automatically for those that do not want the pre-commit hook.

I guess my thought on having guidelines is that if we want them to be followed, and have to setup per-repo linting, etc. everytime, why not have it at the organizational level and common for all of them? It removes some work on a per-repo basis and we can just update all of our repos at once with changes at the organization level, and ensures consistency in a way that having every repo be self-contained does not.

[jacobbieker]

One example being with we should update the organization one to use ruff instead of other things we are using right now, but since its organization-wide, we just have to change it in one place rather than in all the repos we have.

[simlmx]

TLDR If we want globally defined linting, we need to provide tools that can be installed to ensure that the repo is self-contained (not relying on external code that can change and break the CI), and I'm not sure it's worth the investment in time just yet.

I agree with what you are saying in principle but in practice it's very hard to setup properly.

There should be a simple command that we can run in the repo to make sure it passes the lint. For the same reason that we want a command to run the tests in the repo. Otherwise it's too much time spent trying to understand what are the rules that are being enforced by the externally defined CI (the .github repo or by the pre-commit hook, I'm not sure where the latter is defined actually!). One solution would be to have the github workflow come with an executable that lints/formats the code, but I'm not sure it's worth the effort.

Currently the only way I found is to try and copy the global github workflow locally, which means installing the dependencies in a virtualenv (which versions will slowly diverge from the ones used in the workflow), and copy the configuration. If the global lint rules change, then my setup breaks. There is also a chance that I do something slightly different that still passes the CI. The next person could have a slightly different setup that also passes the CI, but doesn't agree with mine. Maybe there is a better way I'm not aware of?

Some other concerns with the current approach:

• What happens if we want different repos to have different linting configs (in particular, I don't think early stage experimental repos should require having docstrings in all files and functions)
• What happens if we want different rules for different files (e.g. different rules for tests or for automatically generated python code).
• If we decide to change the global lint rules, it means all the repos will break without changing anything to their code. To me this in itself is a dealbreaker. The alternative is to version our lint rules but I'm not sure it's worth it.
• The current github workflow is broken (the black checks run in-place instead of failing) and for the same reason as the previous point, fixing it would break all the repos.
• When the linting is defined in the repo you can set up your editor to use it to format your code or show warnings.