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

dx: Add additional hooks to pre-commit #7

Open
wants to merge 7 commits into
base: 202449-pre-commit
Choose a base branch
from

Conversation

s373nZ
Copy link

@s373nZ s373nZ commented Nov 29, 2024

Adds additional hooks to pre-commit.

  • The official shellcheck hook.
  • Re-implements make check-amount-access as a local pygrep hook.
  • Implements a clang-format hook to fail on suggestion warnings with additional configuration to sort includes.
  • Re-implements make check-discouraged-functions as a local pygrep hook.

Adding this PR (along with future hooks to come) against your PR for now, until ElementsProject#7884 is merged.

@s373nZ
Copy link
Author

s373nZ commented Nov 30, 2024

Added an implementation of the make check-amount-access check. Demonstrates a locally defined hook using pygrep and therefore the regex has been converted from piped grep commands into Python format.

@s373nZ
Copy link
Author

s373nZ commented Dec 1, 2024

Added an initial version of a clang-format check. Default behavior is to do in-place editing of the files, but that is overridden and suggestion warnings are reported as errors to force a failure. Output in this fashion isn't exactly easy to see the full context of the suggestion, but one can run the full checks and inspect a diff for more of an overview.

Noticed an existing config in .clang-format and added additional instruction to sort #includes in hopes of eventually replacing make check-src-include-order and make check-hdr-include-order.

Also probably needs some more attention around which files to exclude, in accordance with the SRC_TO_CHECK and ALL_NONGEEN_HEADERS definitions in the Makefile.

@s373nZ
Copy link
Author

s373nZ commented Dec 1, 2024

Added an implementation of the make check-discouraged-functions check, also as a Python regex. There might be another static analysis tool that could also do this (and more?).

@s373nZ s373nZ force-pushed the 7765-pre-commit-hooks branch from b531f46 to 41003ee Compare December 16, 2024 18:14
@s373nZ
Copy link
Author

s373nZ commented Dec 16, 2024

  • Cleaned up regex syntax for earlier hooks.
  • Added codespell to check spelling.

@s373nZ
Copy link
Author

s373nZ commented Dec 17, 2024

  • Added check-jsonschema hooks to validate doc/ JSON schemas and native pre-commit hook for pretty formatting.

Reimplements `make check-amount-access` for Python regex.
Also intends to replace `make check-src-include-order` and
`make check-hdr-include-order` by adding configuration to check
for include ordering.
Reimplements `make check-discouraged-functions` for Python regex.
Includes default config file and an initial word list to ignore.
@s373nZ s373nZ force-pushed the 7765-pre-commit-hooks branch from e5560cd to 3265aea Compare December 18, 2024 19:35
@s373nZ
Copy link
Author

s373nZ commented Dec 18, 2024

  • Added conventional commit linting for commit messages. Looks like the also gets processed by codespell.

Grabbed the list of "scopes" from top level directory organization, along with a sane perusal of the commit messages from the last few months. Needs some auditing.

This hook utilization requires running pre-commit install --hook-type commit-msg. Would be cool to brainstorm a way to bootstrap the contributor environment with one command, like a Makefile target or script, if possible.

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.

1 participant