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 githooks directory #1044

Closed

Conversation

tcharding
Copy link
Member

@tcharding tcharding commented Jun 7, 2022

Add a githooks directory. Copy the sample pre-commit hook into it. Add a section to the README explaining how to take advantage of the githooks.

The sample pre-commit hook includes checks for trailing whitespace that we are seeing occasionally in PRs.

This is done as part of an ongoing effort to get clippy running on CI. With this PR we can later add clippy commands to the pre-commit githook. Devs can then use the hooks if they wish using

git config --local core.hooksPath githooks/

Credit to @LLFourn for the idea. Please note this PR does not use a hidden directory.

Add a `githooks` directory. Copy the sample pre-commit hook into it.
Add a section to the README explaining how to take advantage of the
githooks.

The sample pre-commit hook includes checks for trailing whitespace that
we are seeing occasionally in PRs.

Done in preparation for adding a clippy githook.
Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK 74e522f

apoelstra added a commit to rust-bitcoin/rust-secp256k1 that referenced this pull request Jun 17, 2022
65186e7 Add githooks (Tobin C. Harding)
6d76bd4 Add clippy to CI (Tobin C. Harding)
9f1ebb9 Allow nonminimal_bool in unit test (Tobin C. Harding)
685444c Use "a".repeats() instead of manual implementation (Tobin C. Harding)
42de876 Allow let_and_return for feature guarded code (Tobin C. Harding)
d64132c Allow missing_safety_doc (Tobin C. Harding)
2cb687f Use to_le_bytes instead of mem::transmute (Tobin C. Harding)
c15b9d2 Remove unneeded explicit reference (Tobin C. Harding)
35d59e7 Remove explicit 'static lifetime (Tobin C. Harding)
1a582db Remove redundant import (Tobin C. Harding)

Pull request description:

  The first 8 patches clear clippy warnings. Next we add a CI job to run clippy. Finally we add a `githooks` directory that includes running clippy, also adds a section to the README on how to use the githooks. This is identical to the text in the [open PR](rust-bitcoin/rust-bitcoin#1044) on `rust-bitcoin` that adds githooks _without_ yet adding clippy.

  **Note**: The new clippy CI job runs and is green :)

ACKs for top commit:
  Kixunil:
    ACK 65186e7
  apoelstra:
    ACK 65186e7

Tree-SHA512: f70a157896ce2a83af8cfc10f2fbacc8f68256ac96ef7dec4d190aa72324b568d2267418eb4fe99099aeda5486957c31070943d7c209973859b7b9290676ccd7
@tcharding
Copy link
Member Author

This patch is now included as part of #1061

@tcharding tcharding closed this Jun 23, 2022
# exit with non-zero status after issuing an appropriate message if
# it wants to stop the commit.
#
# To enable this hook, rename this file to "pre-commit".
Copy link
Contributor

Choose a reason for hiding this comment

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

This file is already named pre-commit.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fail! Someone told me that already at some stage recently while working on the githooks. Thanks man, I fixed it up in the clippy PR.

@tcharding tcharding deleted the 06-07-githooks-pre-commit branch August 5, 2022 02:42
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.

None yet

3 participants