Skip to content

Commit

Permalink
feat(cli): prompt user when landing PR with several commits (#572)
Browse files Browse the repository at this point in the history
And add an option to abort the session if trying to land more than one
commit.

Refs: nodejs/node#40436
  • Loading branch information
logictitans committed Oct 28, 2021
1 parent 99e5cb5 commit 84bed38
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 3 deletions.
6 changes: 6 additions & 0 deletions components/git/land.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ const landActions = {
'other commit messages',
default: false,
type: 'boolean'
},
oneCommitMax: {
describe: 'When run in conjunction with the --yes and --autorebase ' +
'options, will abort the session if trying to land more than one commit',
default: false,
type: 'boolean'
}
};

Expand Down
26 changes: 23 additions & 3 deletions lib/landing_session.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,17 @@ const LINT_RESULTS = {
};

class LandingSession extends Session {
constructor(cli, req, dir,
{ prid, backport, lint, autorebase, fixupAll, checkCI } = {}) {
constructor(cli, req, dir, {
prid, backport, lint, autorebase, fixupAll,
checkCI, oneCommitMax
} = {}) {
super(cli, dir, prid);
this.req = req;
this.backport = backport;
this.lint = lint;
this.autorebase = autorebase;
this.fixupAll = fixupAll;
this.oneCommitMax = oneCommitMax;
this.expectedCommitShas = [];
this.checkCI = !!checkCI;
}
Expand All @@ -40,6 +43,7 @@ class LandingSession extends Session {
args.lint = this.lint;
args.autorebase = this.autorebase;
args.fixupAll = this.fixupAll;
args.oneCommitMax = this.oneCommitMax;
return args;
}

Expand Down Expand Up @@ -349,7 +353,9 @@ class LandingSession extends Session {
}

async final() {
const { cli, owner, repo, upstream, branch, prid } = this;
const {
cli, owner, repo, upstream, branch, prid, oneCommitMax
} = this;

// Check that git rebase/am has been completed.
if (!this.readyToFinal()) {
Expand All @@ -360,6 +366,20 @@ class LandingSession extends Session {
};

const stray = this.getStrayCommits();
if (stray.length > 1) {
const forceLand = await cli.prompt(
'There is more than one commit in the PR. ' +
'Do you still want to land it?',
{ defaultAnswer: !oneCommitMax });

if (!forceLand) {
cli.info(
'Use --fixupAll option, squash the PR manually or land the PR from ' +
'the command line.'
);
process.exit(1);
}
}
const strayVerbose = this.getStrayCommits(true);
const validateCommand = path.join(
__dirname,
Expand Down

0 comments on commit 84bed38

Please sign in to comment.