Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configure pre-commit and pre-commit CI #1286

Merged
merged 8 commits into from
Feb 27, 2023

Conversation

VeckoTheGecko
Copy link
Contributor

@VeckoTheGecko VeckoTheGecko commented Jan 23, 2023

Pre-commit and pre-commit CI allow for the quick and seamless detection (and in some cases auto-fixing) of various code quality issues (aka. pre-commit hooks, full list here).

If precommit is to be used locally:

  • install pre-commit from PYPI/Conda-forge (pip install pre-commit, or conda install -c conda-forge pre-commit)
  • run pre-commit install to set up the hooks listed in .pre-commit-config.yaml

Now when you commit, the hooks will be run before the commit is executed (aborting the commit if the hooks pick up errors they can't autofix).

For pre-commit CI, once configured it works automatically on PRs using the same .pre-commit-config.yaml. It commits to the branch for automatic fixes, and gives informative logs for changes it can't autofix.

This pre-commit workflow would replace GitHub actions for linting workflows.

Current hooks:

  • trailing-whitespace
  • end-of-file-fixer
  • check-yaml
  • check-ast (checks files parse as valid Python)
  • flake8

Potential additional hooks:

Configuration of pre-commit CI:

  • Go to pre-commit.ci, sign in with GitHub, and authorize on this repo

Fixes #1285

@VeckoTheGecko VeckoTheGecko marked this pull request as draft January 23, 2023 02:39
@VeckoTheGecko
Copy link
Contributor Author

Converted to draft (waiting on outcome of #1283)

@VeckoTheGecko VeckoTheGecko marked this pull request as ready for review January 23, 2023 08:36
@VeckoTheGecko
Copy link
Contributor Author

Is this something that would be welcome in Parcels? If so, should we add pre_commit as a dev dependency so people can set up the hooks locally?

@erikvansebille
Copy link
Member

Is this something that would be welcome in Parcels? If so, should we add pre_commit as a dev dependency so people can set up the hooks locally?

I think the option of using the pre-commit workflow is nice! I just tried it and it works smoothly. So yes, let's add pre-commit to the environment files. Do you want to do that, @VeckoTheGecko?

@VeckoTheGecko
Copy link
Contributor Author

Do you want to do that, @VeckoTheGecko?

Happy to do so. Will do it within the hour

@VeckoTheGecko
Copy link
Contributor Author

Should I also remove the flake8 workflow @erikvansebille ? If we're using pre-commit CI, then we'd have doubleup with the flake8 linting (GitHub actions + precommit CI)

@erikvansebille erikvansebille merged commit cac7ed0 into OceanParcels:master Feb 27, 2023
@VeckoTheGecko VeckoTheGecko deleted the pre-commit branch June 7, 2023 06:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pre-commit hooks
2 participants