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

Clean and update git hooks #92

Merged
merged 5 commits into from
Jan 10, 2023
Merged

Conversation

Guts
Copy link
Contributor

@Guts Guts commented Jan 3, 2023

Hi there,

First of all, thanks for your great package, it's really useful and well thought-out :).

I saw that you have a configuration for git hooks through pre-commit framework, but it seems to be a bit unmaintained or unused. So, in this PR:

  • ordered hooks A-Z
  • upgraded hooks (using pre-commit autoupdate)
  • clean options
  • added configuration for https://pre-commit.ci - that I suggest your should use for your repo, it's made by the pre-commit author (@asottile) and it's really convenient!

See #92 (comment) for more details.

Copy link
Collaborator

@iamtekson iamtekson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Guts, Thank you very much for your update on git hooks. To be honest, I don't know much about git hooks. The previous implementation was done by @Zeitsperre.

@Zeitsperre, could you please review this PR?

@Guts
Copy link
Contributor Author

Guts commented Jan 6, 2023

Hi @iamtekson

In a nutshell, git hooks are scripts which are launched before/after a git operation, generally the git commit (so the name). It's not an additional tool but one of those included with git: have a look to the .git/hooks subfolder in one of your local repo.

pre-commit is a Python micro-framework to easily manage hooks and handle different languages to code them (Python, rust, node, ruby, etc.). On the nodejs side of the force, you would find husky.

Concretely, git hooks are a really good mechanism to help keeping the code base clean and homogeneous.

You can try it on this repository:

python -m pip install pre-commit
pre-commit install
pre-commit run -a

That should give something like this:

❯ pre-commit run -a
pyupgrade................................................................Failed
- hook id: pyupgrade
- exit code: 1
- files were modified by this hook

Rewriting geo/__version__.py
Rewriting geo/Geoserver.py
Rewriting setup.py

check yaml...............................................................Passed
debug statements (python)................................................Passed
fix end of files.........................................................Failed
- hook id: end-of-file-fixer
- exit code: 1
- files were modified by this hook

Fixing setup.cfg
Fixing .pre-commit-config.yaml
Fixing docs/source/geo.rst

trim trailing whitespace.................................................Failed
- hook id: trailing-whitespace
- exit code: 1
- files were modified by this hook

Fixing .readthedocs.yaml
Fixing docs/source/change_log.rst

black....................................................................Failed
- hook id: black
- files were modified by this hook

reformatted docs/source/conf.py
reformatted setup.py
reformatted tests/test_geoserver.py
reformatted geo/Geoserver.py

All done! ✨ 🍰 ✨
4 files reformatted, 6 files left unchanged.

isort....................................................................Failed
- hook id: isort
- files were modified by this hook

Fixing /home/jmo/Git/External/geoserver-rest/tests/test_geoserver.py

flake8...................................................................Passed
pydocstyle...............................................................Passed
Check hooks apply to the repository......................................Passed
Check for useless excludes...............................................Passed

Next steps:

  • commit those changes
  • add pre-commit.ci service to automatically check against pull request
  • add to the developer documentation (contributing guidelines...)

@Zeitsperre
Copy link
Collaborator

Hello @iamtekson and @Guts, I'm off work for another couple of days, but I can give this PR a look very soon!

Copy link
Collaborator

@Zeitsperre Zeitsperre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from one issue, looks good to me!

.pre-commit-config.yaml Outdated Show resolved Hide resolved
@Zeitsperre
Copy link
Collaborator

@iamtekson This is good for merge IMO. In order to get the pre-commit CI hooks running, you simply need to give permissions to the repo at https://pre-commit.ci/. I use this service in many of my company's repositories, and it's fantastic!

@iamtekson iamtekson merged commit 96c6949 into gicait:master Jan 10, 2023
@Guts Guts deleted the tooling/upgrade-git-hooks branch January 10, 2023 18:41
@Guts Guts mentioned this pull request Jan 10, 2023
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.

3 participants