Skip to content

Commit 9c3ca20

Browse files
chore(NA): removes auto install of pre-commit hook (#83566)
* chore(NA): remove kibana pre-commit hook installation from bootstrap * chore(NA): add support for git ref flag on run precommit hook script * chore(NA): integrate quick commit checks within the CI * chore(NA): introduce logging trap to warn about quick commit checks failure and how to reproduce it * chore(NA): update quick commit checks message * fix(NA): quick commit checks function def * chore(NA): fix quick commit checks message quotes * chore(NA): fix functional call * chore(NA): fix script to run * chore(NA): add unexpected debugger statement to test quick commit checks * chore(NA): update message to log before quick commit checks * chore(NA): remove extra debugger statement * chore(NA): add echo message inline with script execution * chore(NA): add unexpected debugger statement to test quick commit checks * chore(NA): remove extra usage of debug statement * chore(NA): wrapping quick commit checks in a func * chore(NA): export function to use later * chore(NA): export function to use later * chore(NA): use child bash script on github checks reporter * chore(NA): define dir context for commit_check_runner.sh * fix(NA): permissions for commit_check_runner.sh * fix(NA): permissions for commit.sh * chore(NA): format message to log * chore(NA): add unexpected debugger statement to test quick commit checks * chore(NA): remove extra usage of debug statement * chore(NA): format runner message * chore(NA): replace log.info by log.warning * docs(NA): include docs for removing the pre-commit hook auto installation Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
1 parent a6bd1aa commit 9c3ca20

File tree

7 files changed

+64
-8
lines changed

7 files changed

+64
-8
lines changed

docs/developer/getting-started/index.asciidoc

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,20 @@ View all available options by running `yarn start --help`
110110

111111
Read about more advanced options for <<running-kibana-advanced>>.
112112

113+
[discrete]
114+
=== Install pre-commit hook (optional)
115+
116+
In case you want to run a couple of checks like linting or check the file casing of the files to commit, we provide
117+
a way to install a pre-commit hook. To configure it you just need to run the following:
118+
119+
[source,bash]
120+
----
121+
node scripts/register_git_hook
122+
----
123+
124+
After the script completes the pre-commit hook will be created within the file `.git/hooks/pre-commit`.
125+
If you choose to not install it, don't worry, we still run a quick ci check to provide feedback earliest as we can about the same checks.
126+
113127
[discrete]
114128
=== Code away!
115129

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@
6565
"kbn:watch": "node scripts/kibana --dev --logging.json=false",
6666
"build:types": "rm -rf ./target/types && tsc --p tsconfig.types.json",
6767
"docs:acceptApiChanges": "node --max-old-space-size=6144 scripts/check_published_api_changes.js --accept",
68-
"kbn:bootstrap": "node scripts/build_ts_refs && node scripts/register_git_hook",
68+
"kbn:bootstrap": "node scripts/build_ts_refs",
6969
"spec_to_console": "node scripts/spec_to_console",
7070
"backport-skip-ci": "backport --prDescription \"[skip-ci]\"",
7171
"storybook": "node scripts/storybook",

src/dev/precommit_hook/get_files_for_commit.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,13 @@ import { File } from '../file';
2727
* Get the files that are staged for commit (excluding deleted files)
2828
* as `File` objects that are aware of their commit status.
2929
*
30-
* @param {String} repoPath
30+
* @param {String} gitRef
3131
* @return {Promise<Array<File>>}
3232
*/
33-
export async function getFilesForCommit() {
33+
export async function getFilesForCommit(gitRef) {
3434
const simpleGit = new SimpleGit(REPO_ROOT);
35-
36-
const output = await fcb((cb) => simpleGit.diff(['--name-status', '--cached'], cb));
35+
const gitRefForDiff = gitRef ? gitRef : '--cached';
36+
const output = await fcb((cb) => simpleGit.diff(['--name-status', gitRefForDiff], cb));
3737

3838
return (
3939
output

src/dev/run_precommit_hook.js

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,30 @@
1717
* under the License.
1818
*/
1919

20-
import { run, combineErrors } from '@kbn/dev-utils';
20+
import { run, combineErrors, createFlagError } from '@kbn/dev-utils';
2121
import * as Eslint from './eslint';
2222
import * as Sasslint from './sasslint';
2323
import { getFilesForCommit, checkFileCasing } from './precommit_hook';
2424

2525
run(
2626
async ({ log, flags }) => {
27-
const files = await getFilesForCommit();
27+
const files = await getFilesForCommit(flags.ref);
2828
const errors = [];
2929

30+
const maxFilesCount = flags['max-files']
31+
? Number.parseInt(String(flags['max-files']), 10)
32+
: undefined;
33+
if (maxFilesCount !== undefined && (!Number.isFinite(maxFilesCount) || maxFilesCount < 1)) {
34+
throw createFlagError('expected --max-files to be a number greater than 0');
35+
}
36+
37+
if (maxFilesCount && files.length > maxFilesCount) {
38+
log.warning(
39+
`--max-files is set to ${maxFilesCount} and ${files.length} were discovered. The current script execution will be skipped.`
40+
);
41+
return;
42+
}
43+
3044
try {
3145
await checkFileCasing(log, files);
3246
} catch (error) {
@@ -52,15 +66,18 @@ run(
5266
},
5367
{
5468
description: `
55-
Run checks on files that are staged for commit
69+
Run checks on files that are staged for commit by default
5670
`,
5771
flags: {
5872
boolean: ['fix'],
73+
string: ['max-files', 'ref'],
5974
default: {
6075
fix: false,
6176
},
6277
help: `
6378
--fix Execute eslint in --fix mode
79+
--max-files Max files number to check against. If exceeded the script will skip the execution
80+
--ref Run checks against any git ref files (example HEAD or <commit_sha>) instead of running against staged ones
6481
`,
6582
},
6683
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
#!/usr/bin/env bash
2+
3+
source src/dev/ci_setup/setup_env.sh
4+
5+
# Runs pre-commit hook script for the files touched in the last commit.
6+
# That way we can ensure a set of quick commit checks earlier as we removed
7+
# the pre-commit hook installation by default.
8+
# If files are more than 200 we will skip it and just use
9+
# the further ci steps that already check linting and file casing for the entire repo.
10+
checks-reporter-with-killswitch "Quick commit checks" \
11+
"$(dirname "${0}")/commit_check_runner.sh"
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
#!/usr/bin/env bash
2+
3+
run_quick_commit_checks() {
4+
echo "!!!!!!!! ATTENTION !!!!!!!!
5+
That check is intended to provide earlier CI feedback after we remove the automatic install for the local pre-commit hook.
6+
If you want, you can still manually install the pre-commit hook locally by running 'node scripts/register_git_hook locally'
7+
!!!!!!!!!!!!!!!!!!!!!!!!!!!
8+
"
9+
10+
node scripts/precommit_hook.js --ref HEAD~1..HEAD --max-files 200 --verbose
11+
}
12+
13+
run_quick_commit_checks

vars/tasks.groovy

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ def call(List<Closure> closures) {
44

55
def check() {
66
tasks([
7+
kibanaPipeline.scriptTask('Quick Commit Checks', 'test/scripts/checks/commit/commit.sh'),
78
kibanaPipeline.scriptTask('Check Telemetry Schema', 'test/scripts/checks/telemetry.sh'),
89
kibanaPipeline.scriptTask('Check TypeScript Projects', 'test/scripts/checks/ts_projects.sh'),
910
kibanaPipeline.scriptTask('Check Jest Configs', 'test/scripts/checks/jest_configs.sh'),

0 commit comments

Comments
 (0)