Skip to content

Commit

Permalink
git-node: refuse to run without configurations/on wrong revs (#200)
Browse files Browse the repository at this point in the history
It now refuses to run if

1. User has not configured upstream or branch - no more defaults.
2. User is not on the branch configured
3. User is on detached HEAD

Refs: nodejs/node#18914
Fixes: nodejs/node-core-utils#198
  • Loading branch information
gerkai committed Mar 5, 2018
1 parent 859b932 commit b75b854
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 9 deletions.
6 changes: 6 additions & 0 deletions components/git/epilogue.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@
module.exports = `Steps to land a pull request:
==============================================================================
$ cd path/to/node/project
# If you have not configured it before
$ ncu-config set upstream <name-of-remote-to-nodejs/node>
$ ncu-config set branch master # Assuming you are landing commits on master
$ git checkout master
$ git node land --abort # Abort a landing session, just in case
$ git node land $PRID # Start a new landing session
Expand Down
3 changes: 3 additions & 0 deletions components/git/land.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,9 @@ module.exports = {

async function main(state, argv, cli, req, dir) {
let session = new LandingSession(cli, req, dir);
if (session.warnForMissing() || session.warnForWrongBranch()) {
return;
}

try {
session.restore();
Expand Down
12 changes: 7 additions & 5 deletions docs/git-node.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,7 @@ A custom Git command for managing pull requests. You can run it as
1. See the readme on how to
[set up credentials](../README.md#setting-up-credentials).
1. It's a Git command, so make sure you have Git installed, of course.
1. Configure your upstream remote and branch name. By default it assumes your
remote pointing to https://github.com/nodejs/node is called `upstream`, and
the branch that you are trying to land PRs on is `master`. If that's not the
case:

1. Configure your upstream remote and branch name.
```
$ cd path/to/node/project
$ ncu-config set upstream your-remote-name
Expand All @@ -28,6 +24,12 @@ A custom Git command for managing pull requests. You can run it as
Steps to land a pull request:
==============================================================================
$ cd path/to/node/project
# If you have not configured it before
$ ncu-config set upstream <name-of-remote-to-nodejs/node>
$ ncu-config set branch master # Assuming you are landing commits on master
$ git checkout master
$ git node land --abort # Abort a landing session, just in case
$ git node land $PRID # Start a new landing session
Expand Down
61 changes: 59 additions & 2 deletions lib/landing_session.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,14 +138,22 @@ class LandingSession extends Session {
// TODO: do git rebase automatically?
}

getCurrentRev() {
return runSync('git', ['rev-parse', 'HEAD']).trim();
}

getCurrentBranch() {
return runSync('git', ['rev-parse', '--abbrev-ref', 'HEAD']).trim();
}

async amend() {
const { cli } = this;
if (!this.readyToAmend()) {
cli.warn('Not yet ready to amend, run `git node land --abort`');
return;
}

const rev = runSync('git', ['rev-parse', 'HEAD']);
const rev = this.getCurrentRev();
const original = runSync('git', [
'show', 'HEAD', '-s', '--format=%B'
]).trim();
Expand Down Expand Up @@ -311,11 +319,60 @@ class LandingSession extends Session {

const branchName = `${upstream}/${branch}`;
const shouldResetHead = await cli.prompt(
`Do you want to try reset the branch to ${branchName}?`);
`Do you want to try reset the local ${branch} branch to ${branchName}?`);
if (shouldResetHead) {
await this.tryResetHead();
}
}

warnForMissing() {
const { upstream, branch, cli } = this;
const missing = !upstream || !branch;
if (!branch) {
cli.warn('You have not told git-node what branch you are trying' +
' to land commits on.');
cli.separator();
cli.info(
'For example, if your want to land commits on the ' +
'`master` branch, you can run:\n\n' +
' $ ncu-config set branch master');
cli.separator();
}
if (!upstream) {
cli.warn('You have not told git-node the remote you want to sync with.');
cli.separator();
cli.info(
'For example, if your remote pointing to nodejs/node is' +
' `remote-upstream`, you can run:\n\n' +
' $ ncu-config set upstream remote-upstream');
cli.separator();
}
return missing;
}

warnForWrongBranch() {
const { branch, cli } = this;
let rev = this.getCurrentBranch();
if (rev === 'HEAD') {
cli.warn(
'You are in detached HEAD state. Please run git-node on a valid ' +
'branch');
return true;
}
if (rev === branch) {
return false;
}
cli.warn(
`You are running git-node-land on \`${rev}\`,\n but you have` +
` configured \`${branch}\` to be the branch to land commits.`);
cli.separator();
cli.info(
`You can switch to \`${branch}\` with \`git checkout ${branch}\`, or\n` +
` reconfigure the target branch with:\n\n` +
` $ ncu-config set branch ${rev}`);
cli.separator();
return true;
}
}

module.exports = LandingSession;
4 changes: 2 additions & 2 deletions lib/session.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,11 @@ class Session {
}

get upstream() {
return this.config.upstream || 'upstream';
return this.config.upstream;
}

get branch() {
return this.config.branch || 'master';
return this.config.branch;
}

get pullName() {
Expand Down

0 comments on commit b75b854

Please sign in to comment.