Skip to content
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

feat: make lint check opt-in #487

Merged
merged 1 commit into from
Aug 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions components/git/land.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ const landOptions = {
default: false,
type: 'boolean'
},
lint: {
describe: 'Run linter while landing commits',
default: false,
type: 'boolean'
},
autorebase: {
describe: 'Automatically rebase branches with multiple commits',
default: false,
Expand Down Expand Up @@ -99,8 +104,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 @@ -171,7 +176,7 @@ async function main(state, argv, cli, req, dir) {
return;
}
session = new LandingSession(cli, req, dir, argv.prid, argv.backport,
argv.autorebase);
argv.lint, argv.autorebase);
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,17 +15,25 @@ 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, autorebase) {
constructor(cli, req, dir, prid, backport, lint, autorebase) {
super(cli, dir, prid);
this.req = req;
this.backport = backport;
this.lint = lint;
this.autorebase = autorebase;
}

get argv() {
const args = super.argv;
args.backport = this.backport;
args.lint = this.lint;
args.autorebase = this.autorebase;
return args;
}
Expand Down Expand Up @@ -147,14 +155,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 @@ -229,13 +241,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 @@ -253,7 +265,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