-
Notifications
You must be signed in to change notification settings - Fork 21
chore(ci): prevent commit including generated files #379
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
Changes from all commits
3e75735
227edcc
9167fc9
b9d1057
ff0b1b8
054c031
3fe4723
40cbd5e
c5d7781
fbfba2b
c8c3c6e
0b07721
4dc5a01
202169c
6d34e74
62c75fe
541a167
a3d66a6
fc73866
958ff64
d5fa011
958c1fa
613fca9
73528a0
b7e752e
bb700fd
32dd38e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
8.0.5.1 | ||
8.0.5.2 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
#!/bin/sh | ||
. "$(dirname "$0")/_/husky.sh" | ||
|
||
./scripts/ci/husky/pre-commit.js |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
// eslint-disable-next-line import/no-commonjs | ||
module.exports = { | ||
patterns: [ | ||
// Ignore the roots and go down the tree by negating hand written files | ||
'clients/**', | ||
'!clients/README.md', | ||
'!clients/**/.openapi-generator-ignore', | ||
|
||
// Java | ||
'!clients/algoliasearch-client-java-2/algoliasearch-core/src/com/algolia/exceptions/*', | ||
'!clients/algoliasearch-client-java-2/algoliasearch-core/src/com/algolia/utils/*', | ||
'clients/algoliasearch-client-java-2/algoliasearch-core/com/algolia/utils/echo/EchoResponse*.java', | ||
'!clients/algoliasearch-client-java-2/algoliasearch-core/com/algolia/utils/echo/EchoResponseInterface.java', | ||
|
||
// JavaScript | ||
'!clients/algoliasearch-client-javascript/*', | ||
'!clients/algoliasearch-client-javascript/.github/**', | ||
'!clients/algoliasearch-client-javascript/.yarn/**', | ||
'!clients/algoliasearch-client-javascript/scripts/**', | ||
'!clients/algoliasearch-client-javascript/packages/algoliasearch/**', | ||
'!clients/algoliasearch-client-javascript/packages/requester-*/**', | ||
'!clients/algoliasearch-client-javascript/packages/client-common/**', | ||
|
||
// PHP | ||
'!clients/algoliasearch-client-php/lib/Configuration/*', | ||
'clients/algoliasearch-client-php/lib/*.php', | ||
'clients/algoliasearch-client-php/lib/Api/*', | ||
'clients/algoliasearch-client-php/lib/Configuration/Configuration.php', | ||
], | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
/* eslint-disable import/no-commonjs */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Test can still be in TS 😇 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (joking) |
||
// eslint-disable-next-line @typescript-eslint/no-var-requires | ||
const { createMemoizedMicromatchMatcher } = require('../pre-commit'); | ||
|
||
describe('createMemoizedMicromatchMatcher', () => { | ||
it('matches correctly', () => { | ||
const matcher = createMemoizedMicromatchMatcher([ | ||
'clients/**', | ||
'!clients/README.md', | ||
]); | ||
|
||
expect(matcher('clients/README.md')).toEqual(false); | ||
expect(matcher('clients/CONTRIBUTING.md')).toEqual(true); | ||
}); | ||
|
||
it('prioritizes the exact match when two patterns conflict', () => { | ||
const matcher = createMemoizedMicromatchMatcher([ | ||
'!lib/Configuration/*', | ||
'lib/Configuration/Configuration.php', | ||
]); | ||
eunjae-lee marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
expect(matcher('lib/Configuration/Configuration.php')).toEqual(true); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
#!/usr/bin/env node | ||
eunjae-lee marked this conversation as resolved.
Show resolved
Hide resolved
|
||
/* eslint-disable no-console */ | ||
/* eslint-disable import/no-commonjs */ | ||
/* eslint-disable @typescript-eslint/no-var-requires */ | ||
const chalk = require('chalk'); | ||
const execa = require('execa'); | ||
const micromatch = require('micromatch'); | ||
|
||
const GENERATED_FILE_PATTERNS = | ||
require('../../../config/generation.config').patterns; | ||
|
||
const run = async (command, { cwd } = {}) => { | ||
return ( | ||
(await execa.command(command, { shell: 'bash', all: true, cwd })).all ?? '' | ||
); | ||
}; | ||
eunjae-lee marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
function createMemoizedMicromatchMatcher(patterns = []) { | ||
eunjae-lee marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const exactMatchers = []; | ||
const positiveMatchers = []; | ||
const negativeMatchers = []; | ||
|
||
patterns.forEach((pattern) => { | ||
if (pattern.startsWith('!')) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure this simplification of splitting negative and positive will work for every case, for example if we have a sequence of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I'm not too sure either. But I couldn't think of any edge case so far. Once we face it in the future, it will reveal itself and still won't ruin anything. Then we can adjust the logic to cover the case. |
||
// Patterns starting with `!` are negated | ||
negativeMatchers.push(micromatch.matcher(pattern.slice(1))); | ||
} else if (!pattern.includes('*')) { | ||
exactMatchers.push(micromatch.matcher(pattern)); | ||
} else { | ||
positiveMatchers.push(micromatch.matcher(pattern)); | ||
} | ||
}); | ||
|
||
return function matcher(str) { | ||
if (exactMatchers.some((match) => match(str))) { | ||
return true; | ||
} | ||
|
||
// As `some` returns false on empty array, test will always fail if we only | ||
// provide `negativeMatchers`. We fallback to `true` is it's the case. | ||
const hasPositiveMatchers = | ||
Boolean(positiveMatchers.length === 0 && negativeMatchers.length) || | ||
positiveMatchers.some((match) => match(str)); | ||
|
||
return hasPositiveMatchers && !negativeMatchers.some((match) => match(str)); | ||
}; | ||
} | ||
|
||
async function preCommit() { | ||
const stagedFiles = (await run(`git diff --name-only --cached`)).split('\n'); | ||
const deletedFiles = new Set( | ||
(await run(`git ls-files --deleted`)).split('\n') | ||
); | ||
const matcher = createMemoizedMicromatchMatcher(GENERATED_FILE_PATTERNS); | ||
|
||
for (const stagedFile of stagedFiles) { | ||
// keep the deleted files staged even if they were generated before. | ||
if (deletedFiles.has(stagedFile)) { | ||
continue; | ||
} | ||
|
||
if (!matcher(stagedFile)) { | ||
continue; | ||
} | ||
|
||
console.log( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. really nice output 😃 |
||
chalk.bgYellow('[INFO]'), | ||
`Generated file found, unstaging: ${stagedFile}` | ||
); | ||
|
||
await run(`git restore --staged ${stagedFile}`); | ||
} | ||
} | ||
|
||
if (require.main === module && !process.env.CI) { | ||
preCommit(); | ||
} | ||
|
||
module.exports = { | ||
createMemoizedMicromatchMatcher, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
--- | ||
title: Commit and Pull-request | ||
--- | ||
|
||
# Commit and Pull-request | ||
|
||
## Commit | ||
|
||
If you accidentally include generated files in your commit, the `pre-commit` hook will automatically unstage them. | ||
|
||
We create commits on the CI as well, and in that case, we skip this unstaging behavior with the environment variable `CI=true` given. | ||
|
||
If you want to change the patterns of generated file paths, see [config/generation.config.js](https://github.com/algolia/api-clients-automation/blob/main/config/generation.config.js). | ||
|
||
## Pull-request | ||
|
||
Semantic title is required. It's validated by [GitHub Action](https://github.com/deepakputhraya/action-pr-title). See [pr-title.yml](https://github.com/algolia/api-clients-automation/blob/main/.github/workflows/pr-title.yml) for the complete regular expressions. |
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can maybe add a description of what this file does exactly