Skip to content

Commit 9ff409c

Browse files
committed
feat: make lint check opt-in
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: #484
1 parent e379e9f commit 9ff409c

File tree

2 files changed

+28
-10
lines changed

2 files changed

+28
-10
lines changed

components/git/land.js

+9-3
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,11 @@ const landOptions = {
4747
describe: 'Prevent adding Fixes and Refs information to commit metadata',
4848
default: false,
4949
type: 'boolean'
50+
},
51+
lint: {
52+
describe: 'Run linter while landing commits',
53+
default: false,
54+
type: 'boolean'
5055
}
5156
};
5257

@@ -94,8 +99,8 @@ function handler(argv) {
9499

95100
const provided = [];
96101
for (const type of Object.keys(landOptions)) {
97-
// --yes and --skipRefs are not actions.
98-
if (type === 'yes' || type === 'skipRefs') continue;
102+
// Those are not actions.
103+
if (['yes', 'skipRefs', 'lint'].includes(type)) continue;
99104
if (argv[type]) {
100105
provided.push(type);
101106
}
@@ -165,7 +170,8 @@ async function main(state, argv, cli, req, dir) {
165170
cli.log('run `git node land --abort` before starting a new session');
166171
return;
167172
}
168-
session = new LandingSession(cli, req, dir, argv.prid, argv.backport);
173+
session = new LandingSession(cli, req, dir, argv.prid, argv.backport,
174+
argv.lint);
169175
const metadata = await getMetadata(session.argv, argv.skipRefs, cli);
170176
if (argv.backport) {
171177
const split = metadata.metadata.split('\n')[0];

lib/landing_session.js

+19-7
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,24 @@ const { shortSha } = require('./utils');
1515

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

18+
const LINT_RESULTS = {
19+
SKIPPED: 'skipped',
20+
FAILED: 'failed',
21+
SUCCESS: 'success'
22+
};
23+
1824
class LandingSession extends Session {
19-
constructor(cli, req, dir, prid, backport) {
25+
constructor(cli, req, dir, prid, backport, lint) {
2026
super(cli, dir, prid);
2127
this.req = req;
2228
this.backport = backport;
29+
this.lint = lint;
2330
}
2431

2532
get argv() {
2633
const args = super.argv;
2734
args.backport = this.backport;
35+
args.lint = this.lint;
2836
return args;
2937
}
3038

@@ -134,14 +142,18 @@ class LandingSession extends Session {
134142
async validateLint() {
135143
// The linter is currently only run on non-Windows platforms.
136144
if (os.platform() === 'win32') {
137-
return true;
145+
return LINT_RESULTS.SKIPPED;
146+
}
147+
148+
if (!this.lint) {
149+
return LINT_RESULTS.SKIPPED;
138150
}
139151

140152
try {
141153
await runAsync('make', ['lint']);
142-
return true;
154+
return LINT_RESULTS.SUCCESS;
143155
} catch {
144-
return false;
156+
return LINT_RESULTS.FAILED;
145157
}
146158
}
147159

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

198210
const cleanLint = await this.validateLint();
199-
if (!cleanLint) {
211+
if (cleanLint === LINT_RESULTS.FAILED) {
200212
const tryFixLint = await cli.prompt(
201213
'Lint failed - try fixing with \'make lint-js-fix\'?');
202214
if (tryFixLint) {
203215
await runAsync('make', ['lint-js-fix']);
204216
const fixed = await this.validateLint();
205-
if (!fixed) {
217+
if (fixed === LINT_RESULTS.FAILED) {
206218
cli.warn('Patch still contains lint errors. ' +
207219
'Please fix manually before proceeding');
208220
}
@@ -220,7 +232,7 @@ class LandingSession extends Session {
220232
'`git node land --continue`.');
221233
process.exit(1);
222234
}
223-
} else {
235+
} else if (cleanLint === LINT_RESULTS.SUCCESS) {
224236
cli.ok('Lint passed cleanly');
225237
}
226238

0 commit comments

Comments
 (0)