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

Add a more self-contained pre-commit hook #155

Merged

Conversation

cjwatson
Copy link
Contributor

@cjwatson cjwatson commented Nov 5, 2021

Please check if the PR fulfills these requirements

  • The commit message follows our guidelines
  • Tests for the changes have been added (N/A)
  • Docs have been added / updated

What kind of change does this PR introduce?

New alternative pre-commit hook, and corresponding documentation.

What is the current behavior? (You can also link to an open issue here)

The previous pre-commit hook declaration, using language: 'script', relied on woke already being installed on the user's path, which is a somewhat awkward requirement compared to most other pre-commit hooks which tend to be self-contained. It also meant that it wasn't possible to pin woke to a particular version on a per-repository basis using something like rev: v0.15.0.

What is the new behavior (if this is a feature change)?

Using language: 'golang' instead means that pre-commit builds a self-contained installation of woke for itself. The only downside is that the go binary on the user's path must be at least version 1.17, which can be a bit annoying (for example, on Ubuntu 20.04 it requires using something like the go snap rather than the .deb). However, this will become less of a problem over time.

In order to avoid a breaking change for people already using the old approach, I added this as a new alternative hook, called woke-from-source.

Add documentation for all this.

Does this PR introduce a breaking change? (What changes might users need to make due to this PR?)

No - the previous hook is left unchanged. Users may wish to migrate to the new hook if it fits their needs better.

This is an alternative to #152.

The previous `pre-commit` hook declaration, using `language: 'script'`,
relied on `woke` already being installed on the user's path, which is a
somewhat awkward requirement compared to most other `pre-commit` hooks
which tend to be self-contained.  It also meant that it wasn't possible
to pin `woke` to a particular version on a per-repository basis using
something like `rev: v0.15.0`.

Using `language: 'golang'` instead means that `pre-commit` builds a
self-contained installation of `woke` for itself.  The only downside is
that the `go` binary on the user's path must be at least version 1.17,
which can be a bit annoying (for example, on Ubuntu 20.04 it requires
using something like the `go` snap rather than the .deb).  However, this
will become less of a problem over time.

In order to avoid a breaking change for people already using the old
approach, I added this as a new alternative hook, called
`woke-from-source`.

Add documentation for all this.
Copy link
Member

@caitlinelfring caitlinelfring left a comment

Choose a reason for hiding this comment

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

This is great, thanks so much for the addition!

@caitlinelfring caitlinelfring merged commit 2058e2c into get-woke:main Nov 5, 2021
@jugmac00 jugmac00 mentioned this pull request Nov 9, 2021
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.

2 participants