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

feat: pre-commit hook #7428

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Lulalaby
Copy link
Contributor

@Lulalaby Lulalaby commented Mar 17, 2025

No more headache when working with tables

Details

Uses: https://typicode.github.io/husky/ (https://github.com/typicode/husky)
Runs on: git commit
Will be skipped on ci-runs
Runs: fix:tables, test:links & test:build

table

(For Colin :D)

Notes

  • It does not prevent a commit if test:links fails. It only outputs that there's an error. But this is intended since the CI should block that. image
  • "prepare": "npx is-ci || npx husky" is-ci will be true if executed on github actions, skipping the husky run if needed
  • git update-index --again is added to .husky/pre-commit to commit the fixed tables
  • We could remove test:build

@Lulalaby Lulalaby marked this pull request as ready for review March 17, 2025 14:19
@Lulalaby Lulalaby requested a review from a team as a code owner March 17, 2025 14:19
@Lulalaby Lulalaby requested review from colinloretz and removed request for a team March 17, 2025 14:19
npm run fix:tables
npm run test:links
npm run test:build
git update-index --again
Copy link
Contributor Author

@Lulalaby Lulalaby Mar 17, 2025

Choose a reason for hiding this comment

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

We might not need to run test:build here in the pre-commit, since it generates a big chunk of text while committing, and we already run that in the CI

@Lulalaby Lulalaby force-pushed the pre-commit-hook/fix-tables branch from 36e73ea to 46c964c Compare March 17, 2025 16:42
Not sure if this is actually needed, doing a separate commit to ensure we can revert this easily.
Copy link
Contributor

@ckohen ckohen left a comment

Choose a reason for hiding this comment

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

lgtm

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