Skip to content
This repository was archived by the owner on Mar 6, 2026. It is now read-only.

Add option to disable automatic PR creation#83

Merged
tbroadley merged 5 commits intotbroadley:masterfrom
madhavarshney:ControlPRCreation
Jul 4, 2018
Merged

Add option to disable automatic PR creation#83
tbroadley merged 5 commits intotbroadley:masterfrom
madhavarshney:ControlPRCreation

Conversation

@madhavarshney
Copy link
Copy Markdown
Contributor

Closes #79.

Comment thread index.js Outdated

await respondToUserInput(
'Are you sure you want to create a pull request with these corrections?',
`Are you sure you want to ${createPR ? 'create a pull request with' : 'push and send'} these corrections?`,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any ideas for better wording?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about:

  1. "Are you sure you want to create a pull request with these corrections?" if createPR is true
  2. "Are you sure you want to push these changes to on <repository/fork owner and name>?" if createPR is false

Copy link
Copy Markdown
Owner

@tbroadley tbroadley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thank you very much for the contribution!

Would you mind copying the output of node index.js --help into the "Usage" section of README.md, and adding a sentence to CHANGELOG.md describing the change (and linking to this PR and your GitHub profile if you feel like it 🙂)?

Comment thread index.js Outdated
name: 'repository', alias: 'r', typeLabel: '<username/repository or URL>', description: 'The repository to spellcheck.',
},
{
name: 'no-pr', type: Boolean, description: 'Do not create the pull request.',
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about "Do not automatically create a pull request on the target repository."?

Comment thread index.js Outdated

await respondToUserInput(
'Are you sure you want to create a pull request with these corrections?',
`Are you sure you want to ${createPR ? 'create a pull request with' : 'push and send'} these corrections?`,
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about:

  1. "Are you sure you want to create a pull request with these corrections?" if createPR is true
  2. "Are you sure you want to push these changes to on <repository/fork owner and name>?" if createPR is false

Comment thread index.js
description: 'Create a pull request with the specified changes.',
description: createPR
? 'Create a pull request with the specified changes.'
: `Push the changes${!quiet ? ' and open the pull request creation page' : ''}.`,
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Comment thread index.js Outdated
await git(clonePath, `push ${repoFullUrl} refs/heads/${branchName}`);

if (await findGithubFile('PULL_REQUEST_TEMPLATE') && !quiet) {
if (!quiet && !createPR) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the user sets the --no-pr and --quiet flags, quiet === true so a PR will be created. That's not the behaviour I would expect.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops...

@madhavarshney
Copy link
Copy Markdown
Contributor Author

@tbroadley - I updated the PR. Please take a look.

@tbroadley
Copy link
Copy Markdown
Owner

Thanks, I'll take a look soon! Hopefully in the next 24 hours.

@madhavarshney
Copy link
Copy Markdown
Contributor Author

@tbroadley - Sorry for disturbing you, but is there anything I need to do?

Copy link
Copy Markdown
Owner

@tbroadley tbroadley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry for taking so long to review this! Looks great.

@tbroadley tbroadley merged commit fab3db4 into tbroadley:master Jul 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants