From b567c1e57acec50abc12c49f51c93837a7ccd5e4 Mon Sep 17 00:00:00 2001
From: Mary Marchini <mmarchini@netflix.com>
Date: Wed, 19 Aug 2020 19:47:43 -0700
Subject: [PATCH] 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: https://github.com/nodejs/node-core-utils/issues/484
---
 components/git/land.js | 11 ++++++++---
 lib/landing_session.js | 26 +++++++++++++++++++-------
 2 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/components/git/land.js b/components/git/land.js
index ec98360a..237ed7d5 100644
--- a/components/git/land.js
+++ b/components/git/land.js
@@ -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,
@@ -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);
     }
@@ -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];
diff --git a/lib/landing_session.js b/lib/landing_session.js
index 3ac29576..d8504d07 100644
--- a/lib/landing_session.js
+++ b/lib/landing_session.js
@@ -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;
   }
@@ -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;
     }
   }
 
@@ -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');
         }
@@ -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');
     }