Skip to content

Commit 868e26d

Browse files
TerrorSquadTerrorSquad
committed
refactor(hooks): remove auto-commit behavior from artifact generation
- generateApiDocs() and generateDeptracImage() no longer commit - Functions now return ArtifactResult with {generated, changed, path?} - Remove shouldSkip() function that detected auto-generated commits - Simplify pre-push hook to just runTests() and handleArtifacts() - Update tests for new return types
1 parent c56b7f2 commit 868e26d

File tree

4 files changed

+144
-163
lines changed

4 files changed

+144
-163
lines changed

‎booster/.husky/pre-push.ts‎

Lines changed: 22 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,10 @@
44
* Pre-push hook - ZX TypeScript implementation
55
*
66
* Runs checks before pushing:
7-
* - Deptrac (Architecture check)
8-
* - PHPUnit (Tests)
9-
* - API Documentation generation
7+
* - Tests (Pest/PHPUnit)
8+
* - Artifact generation (Deptrac image, API docs) - informational only
109
*/
11-
import { $, fs } from 'zx'
10+
import { fs } from 'zx'
1211
import {
1312
GitHook,
1413
isSkipped,
@@ -19,18 +18,6 @@ import {
1918
generateDeptracImage,
2019
} from './shared/index.ts'
2120

22-
const SKIP_COMMIT_MSG = 'chore: update API documentation'
23-
const SKIP_DEPTRAC_MSG = 'chore: update deptrac image'
24-
25-
export async function shouldSkip(): Promise<boolean> {
26-
const lastCommitMsg = (await $`git log -1 --pretty=%B`).stdout.trim()
27-
if (lastCommitMsg.includes(SKIP_COMMIT_MSG) || lastCommitMsg.includes(SKIP_DEPTRAC_MSG)) {
28-
log.info('Skipping pre-push hook for auto-generated commit.')
29-
return true
30-
}
31-
return false
32-
}
33-
3421
export async function runTests(): Promise<boolean> {
3522
if (await fs.pathExists('vendor/bin/pest')) {
3623
log.tool('Pest', 'Running tests...')
@@ -45,32 +32,33 @@ export async function runTests(): Promise<boolean> {
4532
return true
4633
}
4734

48-
export async function handleApiDocs(): Promise<boolean> {
49-
// Allow skipping API docs generation explicitly via env var
50-
if (isSkipped('api_docs')) {
51-
log.info('Skipping API docs generation (SKIP_API_DOCS environment variable set)')
52-
return true
35+
export async function handleArtifacts(): Promise<void> {
36+
// Generate artifacts - these are informational and don't block the push
37+
// Developers should commit these manually if needed
38+
39+
if (!isSkipped('deptrac_image')) {
40+
await generateDeptracImage()
5341
}
5442

55-
try {
56-
await generateApiDocs()
57-
return true
58-
} catch {
59-
log.error('API spec generation failed')
60-
return false
43+
if (!isSkipped('api_docs')) {
44+
try {
45+
const result = await generateApiDocs()
46+
if (result.changed) {
47+
log.warn('API documentation has changed. Consider committing the changes.')
48+
}
49+
} catch {
50+
// API docs generation is optional, don't fail the push
51+
log.warn('API docs generation failed, but continuing with push')
52+
}
6153
}
6254
}
6355

6456
await runHook(GitHook.PrePush, async () => {
65-
if (await shouldSkip()) return true
66-
67-
// Deptrac check is currently disabled
68-
57+
// Run tests - these ARE blocking
6958
if (!(await runTests())) return false
7059

71-
// Generate artifacts (non-blocking failures)
72-
await generateDeptracImage()
73-
if (!(await handleApiDocs())) return false
60+
// Generate artifacts - informational only, don't block push
61+
await handleArtifacts()
7462

7563
return true
7664
})

‎booster/.husky/shared/extras.ts‎

Lines changed: 51 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -2,92 +2,86 @@ import { fs } from 'zx'
22
import { exec, log } from './core.ts'
33

44
/**
5-
* Generate Deptrac image and add to git
5+
* Result of artifact generation
66
*/
7-
export async function generateDeptracImage(): Promise<void> {
7+
export interface ArtifactResult {
8+
generated: boolean
9+
changed: boolean
10+
path?: string
11+
}
12+
13+
/**
14+
* Generate Deptrac architecture diagram
15+
*
16+
* Generates a PNG image showing the dependency graph.
17+
* Does NOT auto-commit - leaves that to the developer or CI.
18+
*/
19+
export async function generateDeptracImage(): Promise<ArtifactResult> {
820
// Check if deptrac is installed
921
if (!(await fs.pathExists('./vendor/bin/deptrac'))) {
10-
return
22+
log.skip('Deptrac not installed, skipping image generation')
23+
return { generated: false, changed: false }
1124
}
1225

1326
try {
14-
// Use graphviz-image formatter to generate PNG directly
27+
log.tool('Deptrac', 'Generating architecture diagram...')
28+
1529
await exec(
1630
['./vendor/bin/deptrac', '--formatter=graphviz-image', '--output=deptrac.png'],
1731
{ type: 'php' },
1832
)
19-
if (await fs.pathExists('./deptrac.png')) {
20-
await exec(['git', 'add', 'deptrac.png'], { quiet: true })
2133

22-
// Check if there are staged changes for the image
23-
try {
24-
await exec(['git', 'diff', '--cached', '--quiet', 'deptrac.png'], { quiet: true })
25-
} catch {
26-
// Changes detected, commit them
27-
await exec(['git', 'commit', '-m', 'chore: update deptrac image'])
28-
log.success('Deptrac image updated and committed')
29-
}
34+
if (await fs.pathExists('./deptrac.png')) {
35+
log.success('Deptrac image generated: deptrac.png')
36+
return { generated: true, changed: true, path: 'deptrac.png' }
3037
}
38+
39+
return { generated: false, changed: false }
3140
} catch (error: unknown) {
32-
// Image generation is optional, don't fail if it doesn't work
3341
const errorMessage = error instanceof Error ? error.message : String(error)
3442
log.warn(`Deptrac image generation failed: ${errorMessage}`)
43+
return { generated: false, changed: false }
3544
}
3645
}
3746

3847
/**
39-
* Generate API documentation if OpenAPI spec has changed
48+
* Generate API documentation from OpenAPI annotations
49+
*
50+
* Generates both the YAML spec and HTML documentation.
51+
* Does NOT auto-commit - leaves that to the developer or CI.
4052
*/
41-
export async function generateApiDocs(): Promise<void> {
53+
export async function generateApiDocs(): Promise<ArtifactResult> {
54+
// Check if swagger-php is installed
55+
if (!(await fs.pathExists('./vendor/bin/openapi'))) {
56+
log.skip('swagger-php not installed, skipping API docs generation')
57+
return { generated: false, changed: false }
58+
}
59+
4260
log.tool('API Documentation', 'Generating OpenAPI specification...')
43-
try {
44-
// Check if swagger-php is installed by looking for the binary
45-
// This avoids reading/parsing composer.lock
46-
if (await fs.pathExists('./vendor/bin/openapi')) {
47-
await exec(['composer', 'generate-api-spec'], { type: 'php' })
4861

49-
const diffResult = await exec(['git', 'diff', '--name-only'], { quiet: true })
50-
const modifiedFiles = diffResult.toString().trim().split('\n')
62+
try {
63+
await exec(['composer', 'generate-api-spec'], { type: 'php' })
64+
log.success('OpenAPI specification generated')
5165

52-
if (modifiedFiles.includes('openapi/openapi.yml')) {
53-
log.info('API spec changed, regenerating HTML...')
66+
// Check if spec changed
67+
const diffResult = await exec(['git', 'diff', '--name-only'], { quiet: true })
68+
const modifiedFiles = diffResult.toString().trim().split('\n')
5469

55-
try {
56-
await exec(['pnpm', 'generate:api-doc:html'])
57-
log.success('HTML documentation generated')
70+
if (modifiedFiles.includes('openapi/openapi.yml')) {
71+
log.info('API spec changed, regenerating HTML...')
5872

59-
// Stage the generated files
60-
await exec(
61-
['git', 'add', 'openapi/openapi.html', 'openapi/openapi.yml'],
62-
{ quiet: true },
63-
)
73+
await exec(['pnpm', 'generate:api-doc:html'])
74+
log.success('HTML documentation generated')
6475

65-
// Check if there are staged changes and commit them
66-
try {
67-
await exec(['git', 'diff', '--cached', '--quiet'], { quiet: true })
68-
// If we get here, there are no staged changes
69-
log.info('No staged changes for API documentation')
70-
} catch {
71-
// There are staged changes, commit them
72-
await exec(['git', 'commit', '-m', 'chore: update API documentation'])
73-
log.success('API documentation committed')
74-
}
75-
} catch (error: unknown) {
76-
const errorMessage = error instanceof Error ? error.message : String(error)
77-
log.error(`HTML documentation generation failed: ${errorMessage}`)
78-
throw error
79-
}
80-
} else {
81-
log.info('No changes to OpenAPI specification, skipping HTML generation')
82-
}
83-
} else {
84-
log.info('swagger-php not installed -> skipping API docs.')
76+
log.info('Artifacts generated. Remember to commit: openapi/openapi.yml, openapi/openapi.html')
77+
return { generated: true, changed: true, path: 'openapi/openapi.yml' }
8578
}
79+
80+
log.info('No changes to OpenAPI specification')
81+
return { generated: true, changed: false }
8682
} catch (error: unknown) {
8783
const errorMessage = error instanceof Error ? error.message : String(error)
88-
log.error(`API spec generation failed: ${errorMessage}`)
89-
// Don't throw, just log error, as this might be optional?
90-
// Original pre-push returned false on error.
84+
log.error(`API documentation generation failed: ${errorMessage}`)
9185
throw error
9286
}
9387
}

‎booster/.husky/tests/pre-push.test.mjs‎

Lines changed: 27 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'
22

33
// Mock dependencies
44
vi.mock('zx', () => ({
5-
$: vi.fn(),
65
fs: {
76
pathExists: vi.fn(),
87
},
@@ -16,6 +15,7 @@ vi.mock('../shared/index.ts', () => ({
1615
tool: vi.fn(),
1716
success: vi.fn(),
1817
error: vi.fn(),
18+
warn: vi.fn(),
1919
},
2020
runHook: vi.fn(), // Prevent automatic execution
2121
exec: vi.fn(),
@@ -24,30 +24,17 @@ vi.mock('../shared/index.ts', () => ({
2424
}))
2525

2626
// Import dependencies for mocking return values
27-
import { $ } from 'zx'
2827
import { fs } from 'zx'
2928
import { exec, generateApiDocs, isSkipped, generateDeptracImage } from '../shared/index.ts'
3029

3130
// Import functions to test
32-
import { shouldSkip, runTests, handleApiDocs } from '../pre-push.ts'
31+
import { runTests, handleArtifacts } from '../pre-push.ts'
3332

3433
describe('Pre-push Hook', () => {
3534
beforeEach(() => {
3635
vi.clearAllMocks()
3736
})
3837

39-
describe('shouldSkip', () => {
40-
it('should skip if commit message contains skip tag', async () => {
41-
$.mockResolvedValue({ stdout: 'chore: update API documentation' })
42-
expect(await shouldSkip()).toBe(true)
43-
})
44-
45-
it('should not skip for normal commits', async () => {
46-
$.mockResolvedValue({ stdout: 'feat: new feature' })
47-
expect(await shouldSkip()).toBe(false)
48-
})
49-
})
50-
5138
describe('runTests', () => {
5239
it('should run tests if pest exists', async () => {
5340
fs.pathExists.mockResolvedValue(true)
@@ -78,39 +65,45 @@ describe('Pre-push Hook', () => {
7865
})
7966
})
8067

81-
describe('handleApiDocs', () => {
82-
it('should generate api docs', async () => {
68+
describe('handleArtifacts', () => {
69+
it('should generate both deptrac image and api docs', async () => {
8370
isSkipped.mockReturnValue(false)
84-
generateApiDocs.mockResolvedValue(true)
71+
generateDeptracImage.mockResolvedValue({ generated: true, changed: true })
72+
generateApiDocs.mockResolvedValue({ generated: true, changed: false })
8573

86-
const result = await handleApiDocs()
74+
await handleArtifacts()
8775

88-
expect(result).toBe(true)
76+
expect(generateDeptracImage).toHaveBeenCalled()
8977
expect(generateApiDocs).toHaveBeenCalled()
9078
})
9179

92-
it('should skip if env var is set', async () => {
93-
isSkipped.mockReturnValue(true)
80+
it('should skip deptrac if env var is set', async () => {
81+
isSkipped.mockImplementation((name) => name === 'deptrac_image')
82+
generateApiDocs.mockResolvedValue({ generated: true, changed: false })
9483

95-
const result = await handleApiDocs()
84+
await handleArtifacts()
9685

97-
expect(result).toBe(true)
98-
expect(generateApiDocs).not.toHaveBeenCalled()
86+
expect(generateDeptracImage).not.toHaveBeenCalled()
87+
expect(generateApiDocs).toHaveBeenCalled()
9988
})
10089

101-
it('should return false if generation fails', async () => {
102-
isSkipped.mockReturnValue(false)
103-
generateApiDocs.mockRejectedValue(new Error('Failed'))
90+
it('should skip api docs if env var is set', async () => {
91+
isSkipped.mockImplementation((name) => name === 'api_docs')
92+
generateDeptracImage.mockResolvedValue({ generated: true, changed: false })
10493

105-
const result = await handleApiDocs()
94+
await handleArtifacts()
10695

107-
expect(result).toBe(false)
96+
expect(generateDeptracImage).toHaveBeenCalled()
97+
expect(generateApiDocs).not.toHaveBeenCalled()
10898
})
109-
})
11099

111-
describe('Integration', () => {
112-
it('should generate deptrac image', async () => {
113-
expect(generateDeptracImage).toBeDefined()
100+
it('should not throw if api docs generation fails', async () => {
101+
isSkipped.mockReturnValue(false)
102+
generateDeptracImage.mockResolvedValue({ generated: true, changed: false })
103+
generateApiDocs.mockRejectedValue(new Error('Failed'))
104+
105+
// Should not throw
106+
await expect(handleArtifacts()).resolves.not.toThrow()
114107
})
115108
})
116109
})

0 commit comments

Comments
 (0)