Skip to content

Comments

Add pre-commit config#23

Open
mikapfl wants to merge 2 commits intoopenclimatedata:mainfrom
mikapfl:pre-commit
Open

Add pre-commit config#23
mikapfl wants to merge 2 commits intoopenclimatedata:mainfrom
mikapfl:pre-commit

Conversation

@mikapfl
Copy link
Contributor

@mikapfl mikapfl commented Aug 2, 2022

Using a pre-commit config together with https://pre-commit.ci/ , we can get rid of all byte-order markers and missing newlines at the end of files and anachronistic python constructs automatically, so we will never again have a bug like #21.

In this PR, I have only included the config itself, not the changes it would generate. If you want to see the changes yourself, run pre-commit run --all-files in a checkout of this branch. If you think it is a good idea, enable https://pre-commit.ci/ for this repo and the changes will be added automatically to the next PR (or maybe already to this PR, I don't know).

@rgieseke
Copy link
Contributor

rgieseke commented Aug 2, 2022

Looks useful, but i'm generally torn between automating clean-up (thank you!) and relying on external services (which i try to avoid if possible).
I need to give this a closer look when i have some time.

@mikapfl
Copy link
Contributor Author

mikapfl commented Aug 3, 2022

Looks useful, but i'm generally torn between automating clean-up (thank you!) and relying on external services (which i try to avoid if possible).

Oh, I understand the idea not to rely on external services! A purely git-based workflow is also possible with pre-commit. Run your flavor of

pip3 install --user pre-commit
cd /path/to/globalwarmingpotentials
pre-commit install

and in the future it will check/fix everything when doing a git commit. If it fixes anything or a check fails, the git commit is rejected. The downside is that every contributor needs to set this up if they want to get the benefits. But you can also just run

pre-commit run --all-files

before each release and have things in a pretty good state.

The pre-commit.ci service is a nice addition on github, but not really necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants