Skip to content

Commit

Permalink
feat: make lint check opt-in
Browse files Browse the repository at this point in the history
Disable lint check by default and make it opt-in, so it doesn't impact
the workflow for collaborators, while providing a way for early birds to
try it out so we can tweak it until it works great for everyone (at
which point we can re-enable it by default).

Ref: nodejs#484
  • Loading branch information
mmarchini committed Aug 20, 2020
1 parent e379e9f commit 9ff409c
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 10 deletions.
12 changes: 9 additions & 3 deletions components/git/land.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ const landOptions = {
describe: 'Prevent adding Fixes and Refs information to commit metadata',
default: false,
type: 'boolean'
},
lint: {
describe: 'Run linter while landing commits',
default: false,
type: 'boolean'
}
};

Expand Down Expand Up @@ -94,8 +99,8 @@ function handler(argv) {

const provided = [];
for (const type of Object.keys(landOptions)) {
// --yes and --skipRefs are not actions.
if (type === 'yes' || type === 'skipRefs') continue;
// Those are not actions.
if (['yes', 'skipRefs', 'lint'].includes(type)) continue;
if (argv[type]) {
provided.push(type);
}
Expand Down Expand Up @@ -165,7 +170,8 @@ async function main(state, argv, cli, req, dir) {
cli.log('run `git node land --abort` before starting a new session');
return;
}
session = new LandingSession(cli, req, dir, argv.prid, argv.backport);
session = new LandingSession(cli, req, dir, argv.prid, argv.backport,
argv.lint);
const metadata = await getMetadata(session.argv, argv.skipRefs, cli);
if (argv.backport) {
const split = metadata.metadata.split('\n')[0];
Expand Down
26 changes: 19 additions & 7 deletions lib/landing_session.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,24 @@ const { shortSha } = require('./utils');

const isWindows = process.platform === 'win32';

const LINT_RESULTS = {
SKIPPED: 'skipped',
FAILED: 'failed',
SUCCESS: 'success'
};

class LandingSession extends Session {
constructor(cli, req, dir, prid, backport) {
constructor(cli, req, dir, prid, backport, lint) {
super(cli, dir, prid);
this.req = req;
this.backport = backport;
this.lint = lint;
}

get argv() {
const args = super.argv;
args.backport = this.backport;
args.lint = this.lint;
return args;
}

Expand Down Expand Up @@ -134,14 +142,18 @@ class LandingSession extends Session {
async validateLint() {
// The linter is currently only run on non-Windows platforms.
if (os.platform() === 'win32') {
return true;
return LINT_RESULTS.SKIPPED;
}

if (!this.lint) {
return LINT_RESULTS.SKIPPED;
}

try {
await runAsync('make', ['lint']);
return true;
return LINT_RESULTS.SUCCESS;
} catch {
return false;
return LINT_RESULTS.FAILED;
}
}

Expand Down Expand Up @@ -196,13 +208,13 @@ class LandingSession extends Session {
const patch = await this.downloadAndPatch();

const cleanLint = await this.validateLint();
if (!cleanLint) {
if (cleanLint === LINT_RESULTS.FAILED) {
const tryFixLint = await cli.prompt(
'Lint failed - try fixing with \'make lint-js-fix\'?');
if (tryFixLint) {
await runAsync('make', ['lint-js-fix']);
const fixed = await this.validateLint();
if (!fixed) {
if (fixed === LINT_RESULTS.FAILED) {
cli.warn('Patch still contains lint errors. ' +
'Please fix manually before proceeding');
}
Expand All @@ -220,7 +232,7 @@ class LandingSession extends Session {
'`git node land --continue`.');
process.exit(1);
}
} else {
} else if (cleanLint === LINT_RESULTS.SUCCESS) {
cli.ok('Lint passed cleanly');
}

Expand Down

0 comments on commit 9ff409c

Please sign in to comment.