v2: Add left-hook pre-push
#1003
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -52,3 +52,5 @@ dist/ | |
|
|
||
| # Conformance test results | ||
| results/ | ||
|
|
||
| .lefthook-local.yml | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| # Optional local lefthook configuration | ||
| # To enable this: | ||
| # cp lefthook-local.example.yml lefthook-local.yml | ||
|
|
||
| pre-commit: | ||
| parallel: true | ||
| jobs: | ||
| - name: 'Typecheck' | ||
| run: pnpm typecheck:all | ||
| stage_fixed: true | ||
|
claude[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
| - name: 'Lint Fix & Format' | ||
| run: pnpm lint:fix:all | ||
| stage_fixed: true | ||
|
|
||
| post-checkout: | ||
| jobs: | ||
| - name: 'Install Dependencies' | ||
| run: pnpm install | ||
|
|
||
| post-merge: | ||
| jobs: | ||
| - name: 'Install Dependencies' | ||
| run: pnpm install | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| # lefthook.yml | ||
| # Configuration reference: https://lefthook.dev/configuration/ | ||
|
|
||
| assert_lefthook_installed: true | ||
|
|
||
| output: | ||
| - meta # Print lefthook version | ||
| - summary # Print summary block (successful and failed steps) | ||
| - empty_summary # Print summary heading when there are no steps to run | ||
| - success # Print successful steps | ||
| - failure # Print failed steps printing | ||
| - execution # Print any execution logs (but prints if the execution failed) | ||
| - execution_out # Print execution output (but still prints failed commands output) | ||
| - execution_info # Print `EXECUTE > ...` logging | ||
| - skips # Print "skip" (i.e. no files matched) | ||
|
|
||
| pre-push: | ||
| follow: true | ||
| parallel: true | ||
| jobs: | ||
| - name: 'Typecheck' | ||
| run: pnpm run typecheck:all | ||
| fail_text: | | ||
| 💡 To catch typechecking issues earlier, enable the pre-commit hook: | ||
| cp lefthook-local.example.yml lefthook-local.yml | ||
|
|
||
| - name: 'Lint' | ||
| run: pnpm run lint:all | ||
| fail_text: | | ||
| 💡 To catch linting issues earlier, enable the pre-commit hook: | ||
| cp lefthook-local.example.yml lefthook-local.yml | ||
|
|
||
| - name: 'Build' | ||
| run: pnpm run build:all | ||
| fail_text: | | ||
| 💡 To catch build issues earlier, enable the pre-commit hook: | ||
| cp lefthook-local.example.yml lefthook-local.yml | ||
|
Comment on lines
+33
to
+37
There was a problem hiding this comment. 🟡 The Build job fail_text (lines 35-37) suggests enabling the pre-commit hook to catch build issues earlier, but lefthook-local.example.yml only defines Typecheck and Lint Fix & Format pre-commit jobs -- there is no build step. A contributor following this advice would still only get build feedback at pre-push time. Extended reasoning...What the bug isThe Build job in lefthook.yml (lines 33-37) has a fail_text that reads:
This suggests that enabling the optional pre-commit hooks will catch build failures earlier. However, lefthook-local.example.yml only defines two pre-commit jobs: Typecheck (pnpm typecheck:all) and Lint Fix & Format (pnpm lint:fix:all). There is no build step in the pre-commit configuration. Step-by-step proof
Why existing code does not prevent thisThe Typecheck and Lint jobs have similar fail_text messages, which are at least partially justified -- the pre-commit config does include pnpm typecheck:all and pnpm lint:fix:all. It appears the Build job fail_text was copied from one of the other jobs without adjusting for the fact that no corresponding build step exists in the pre-commit example. ImpactThis is a misleading hint that could confuse contributors. Someone who follows the advice expecting their build issues to be caught at commit time will be frustrated when they still only see build failures at push time. It erodes trust in the developer tooling suggestions. How to fixEither:
Option 1 is likely preferable since running a full build on every commit would be slow and counterproductive for the DX goals of this PR. |
||
|
|
||
| post-checkout: | ||
| jobs: | ||
| - name: 'Install Dependencies' | ||
| run: pnpm install | ||
|
|
||
| post-merge: | ||
| jobs: | ||
| - name: 'Install Dependencies' | ||
| run: pnpm install | ||
Uh oh!
There was an error while loading. Please reload this page.