Skip to content

Commit

Permalink
chore(NA): removes auto install of pre-commit hook (elastic#83566)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>
  • Loading branch information
mistic and kibanamachine committed Dec 4, 2020
1 parent b63d0a6 commit 2f754ac
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 8 deletions.
14 changes: 14 additions & 0 deletions docs/developer/getting-started/index.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,20 @@ View all available options by running `yarn start --help`

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

[discrete]
=== Install pre-commit hook (optional)

In case you want to run a couple of checks like linting or check the file casing of the files to commit, we provide
a way to install a pre-commit hook. To configure it you just need to run the following:

[source,bash]
----
node scripts/register_git_hook
----

After the script completes the pre-commit hook will be created within the file `.git/hooks/pre-commit`.
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.

[discrete]
=== Code away!

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
"kbn:watch": "node scripts/kibana --dev --logging.json=false",
"build:types": "rm -rf ./target/types && tsc --p tsconfig.types.json",
"docs:acceptApiChanges": "node --max-old-space-size=6144 scripts/check_published_api_changes.js --accept",
"kbn:bootstrap": "node scripts/build_ts_refs && node scripts/register_git_hook",
"kbn:bootstrap": "node scripts/build_ts_refs",
"spec_to_console": "node scripts/spec_to_console",
"storybook": "node scripts/storybook"
},
Expand Down
8 changes: 4 additions & 4 deletions src/dev/precommit_hook/get_files_for_commit.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@ import { File } from '../file';
* Get the files that are staged for commit (excluding deleted files)
* as `File` objects that are aware of their commit status.
*
* @param {String} repoPath
* @param {String} gitRef
* @return {Promise<Array<File>>}
*/
export async function getFilesForCommit() {
export async function getFilesForCommit(gitRef) {
const simpleGit = new SimpleGit(REPO_ROOT);

const output = await fcb((cb) => simpleGit.diff(['--name-status', '--cached'], cb));
const gitRefForDiff = gitRef ? gitRef : '--cached';
const output = await fcb((cb) => simpleGit.diff(['--name-status', gitRefForDiff], cb));

return (
output
Expand Down
23 changes: 20 additions & 3 deletions src/dev/run_precommit_hook.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,30 @@
* under the License.
*/

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

run(
async ({ log, flags }) => {
const files = await getFilesForCommit();
const files = await getFilesForCommit(flags.ref);
const errors = [];

const maxFilesCount = flags['max-files']
? Number.parseInt(String(flags['max-files']), 10)
: undefined;
if (maxFilesCount !== undefined && (!Number.isFinite(maxFilesCount) || maxFilesCount < 1)) {
throw createFlagError('expected --max-files to be a number greater than 0');
}

if (maxFilesCount && files.length > maxFilesCount) {
log.warning(
`--max-files is set to ${maxFilesCount} and ${files.length} were discovered. The current script execution will be skipped.`
);
return;
}

try {
await checkFileCasing(log, files);
} catch (error) {
Expand All @@ -52,15 +66,18 @@ run(
},
{
description: `
Run checks on files that are staged for commit
Run checks on files that are staged for commit by default
`,
flags: {
boolean: ['fix'],
string: ['max-files', 'ref'],
default: {
fix: false,
},
help: `
--fix Execute eslint in --fix mode
--max-files Max files number to check against. If exceeded the script will skip the execution
--ref Run checks against any git ref files (example HEAD or <commit_sha>) instead of running against staged ones
`,
},
}
Expand Down
11 changes: 11 additions & 0 deletions test/scripts/checks/commit/commit.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#!/usr/bin/env bash

source src/dev/ci_setup/setup_env.sh

# Runs pre-commit hook script for the files touched in the last commit.
# That way we can ensure a set of quick commit checks earlier as we removed
# the pre-commit hook installation by default.
# If files are more than 200 we will skip it and just use
# the further ci steps that already check linting and file casing for the entire repo.
checks-reporter-with-killswitch "Quick commit checks" \
"$(dirname "${0}")/commit_check_runner.sh"
13 changes: 13 additions & 0 deletions test/scripts/checks/commit/commit_check_runner.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#!/usr/bin/env bash

run_quick_commit_checks() {
echo "!!!!!!!! ATTENTION !!!!!!!!
That check is intended to provide earlier CI feedback after we remove the automatic install for the local pre-commit hook.
If you want, you can still manually install the pre-commit hook locally by running 'node scripts/register_git_hook locally'
!!!!!!!!!!!!!!!!!!!!!!!!!!!
"

node scripts/precommit_hook.js --ref HEAD~1..HEAD --max-files 200 --verbose
}

run_quick_commit_checks
1 change: 1 addition & 0 deletions vars/tasks.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ def call(List<Closure> closures) {

def check() {
tasks([
kibanaPipeline.scriptTask('Quick Commit Checks', 'test/scripts/checks/commit/commit.sh'),
kibanaPipeline.scriptTask('Check Telemetry Schema', 'test/scripts/checks/telemetry.sh'),
kibanaPipeline.scriptTask('Check TypeScript Projects', 'test/scripts/checks/ts_projects.sh'),
kibanaPipeline.scriptTask('Check Jest Configs', 'test/scripts/checks/jest_configs.sh'),
Expand Down

0 comments on commit 2f754ac

Please sign in to comment.