Skip to content

Commit 629d4d4

Browse files
committed
git-node: refuse to run without configurations/on wrong revs (#200)
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
1 parent 7f6fa8b commit 629d4d4

File tree

5 files changed

+77
-9
lines changed

5 files changed

+77
-9
lines changed

components/git/epilogue.js

+6
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,12 @@
33
module.exports = `Steps to land a pull request:
44
==============================================================================
55
$ cd path/to/node/project
6+
7+
# If you have not configured it before
8+
$ ncu-config set upstream <name-of-remote-to-nodejs/node>
9+
$ ncu-config set branch master # Assuming you are landing commits on master
10+
11+
$ git checkout master
612
$ git node land --abort # Abort a landing session, just in case
713
$ git node land $PRID # Start a new landing session
814

components/git/land.js

+3
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,9 @@ module.exports = {
103103

104104
async function main(state, argv, cli, req, dir) {
105105
let session = new LandingSession(cli, req, dir);
106+
if (session.warnForMissing() || session.warnForWrongBranch()) {
107+
return;
108+
}
106109

107110
try {
108111
session.restore();

docs/git-node.md

+7-5
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,7 @@ A custom Git command for managing pull requests. You can run it as
88
1. See the readme on how to
99
[set up credentials](../README.md#setting-up-credentials).
1010
1. It's a Git command, so make sure you have Git installed, of course.
11-
1. Configure your upstream remote and branch name. By default it assumes your
12-
remote pointing to https://github.com/nodejs/node is called `upstream`, and
13-
the branch that you are trying to land PRs on is `master`. If that's not the
14-
case:
15-
11+
1. Configure your upstream remote and branch name.
1612
```
1713
$ cd path/to/node/project
1814
$ ncu-config set upstream your-remote-name
@@ -28,6 +24,12 @@ A custom Git command for managing pull requests. You can run it as
2824
Steps to land a pull request:
2925
==============================================================================
3026
$ cd path/to/node/project
27+
28+
# If you have not configured it before
29+
$ ncu-config set upstream <name-of-remote-to-nodejs/node>
30+
$ ncu-config set branch master # Assuming you are landing commits on master
31+
32+
$ git checkout master
3133
$ git node land --abort # Abort a landing session, just in case
3234
$ git node land $PRID # Start a new landing session
3335

lib/landing_session.js

+59-2
Original file line numberDiff line numberDiff line change
@@ -138,14 +138,22 @@ class LandingSession extends Session {
138138
// TODO: do git rebase automatically?
139139
}
140140

141+
getCurrentRev() {
142+
return runSync('git', ['rev-parse', 'HEAD']).trim();
143+
}
144+
145+
getCurrentBranch() {
146+
return runSync('git', ['rev-parse', '--abbrev-ref', 'HEAD']).trim();
147+
}
148+
141149
async amend() {
142150
const { cli } = this;
143151
if (!this.readyToAmend()) {
144152
cli.warn('Not yet ready to amend, run `git node land --abort`');
145153
return;
146154
}
147155

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

312320
const branchName = `${upstream}/${branch}`;
313321
const shouldResetHead = await cli.prompt(
314-
`Do you want to try reset the branch to ${branchName}?`);
322+
`Do you want to try reset the local ${branch} branch to ${branchName}?`);
315323
if (shouldResetHead) {
316324
await this.tryResetHead();
317325
}
318326
}
327+
328+
warnForMissing() {
329+
const { upstream, branch, cli } = this;
330+
const missing = !upstream || !branch;
331+
if (!branch) {
332+
cli.warn('You have not told git-node what branch you are trying' +
333+
' to land commits on.');
334+
cli.separator();
335+
cli.info(
336+
'For example, if your want to land commits on the ' +
337+
'`master` branch, you can run:\n\n' +
338+
' $ ncu-config set branch master');
339+
cli.separator();
340+
}
341+
if (!upstream) {
342+
cli.warn('You have not told git-node the remote you want to sync with.');
343+
cli.separator();
344+
cli.info(
345+
'For example, if your remote pointing to nodejs/node is' +
346+
' `remote-upstream`, you can run:\n\n' +
347+
' $ ncu-config set upstream remote-upstream');
348+
cli.separator();
349+
}
350+
return missing;
351+
}
352+
353+
warnForWrongBranch() {
354+
const { branch, cli } = this;
355+
let rev = this.getCurrentBranch();
356+
if (rev === 'HEAD') {
357+
cli.warn(
358+
'You are in detached HEAD state. Please run git-node on a valid ' +
359+
'branch');
360+
return true;
361+
}
362+
if (rev === branch) {
363+
return false;
364+
}
365+
cli.warn(
366+
`You are running git-node-land on \`${rev}\`,\n but you have` +
367+
` configured \`${branch}\` to be the branch to land commits.`);
368+
cli.separator();
369+
cli.info(
370+
`You can switch to \`${branch}\` with \`git checkout ${branch}\`, or\n` +
371+
` reconfigure the target branch with:\n\n` +
372+
` $ ncu-config set branch ${rev}`);
373+
cli.separator();
374+
return true;
375+
}
319376
}
320377

321378
module.exports = LandingSession;

lib/session.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,11 @@ class Session {
4141
}
4242

4343
get upstream() {
44-
return this.config.upstream || 'upstream';
44+
return this.config.upstream;
4545
}
4646

4747
get branch() {
48-
return this.config.branch || 'master';
48+
return this.config.branch;
4949
}
5050

5151
get pullName() {

0 commit comments

Comments
 (0)