Draft PR support #3
Conversation
greglockwood
force-pushed
the
feature/support-drafts
branch
from
d470ef3 to
a0816c3
Compare
greglockwood
changed the title
cfg_template.yml
Outdated
| # Create new PRs in draft mode by default. This will prevent them from notifying anybody | ||
| # until you are ready. This option can be overridden on the command line using the | ||
| # -d/--draft or --no-draft options, which set draft mode to true or false, respectively. | ||
| draft-by-default: true |
There was a problem hiding this comment.
Should this be commented out by default?
There was a problem hiding this comment.
Hmm...I leaning towards yes, commented out by default? I personally don't use draft PRs much - I just don't assign reviewers until the PR is ready.
| @@ -130,7 +135,7 @@ function getBranchesInCurrentTrain(branchConfig) { | |||
| */ | |||
| function getCombinedBranch(branchConfig) { | |||
| const combinedBranch = /** @type {Object<string, {combined: boolean}>} */ branchConfig.find(cfg => { | |||
| if (typeof cfg === 'string') { | |||
| if (!cfg || typeof cfg === 'string') { | |||
There was a problem hiding this comment.
cfg can be nullish if you make the mistake (as I did) of not indenting the combined: true line for the combined branch.
There was a problem hiding this comment.
Awesome work @greglockwood ! I just left a few questions to clarify before we can merge.
cfg_template.yml
Outdated
| # Create new PRs in draft mode by default. This will prevent them from notifying anybody | ||
| # until you are ready. This option can be overridden on the command line using the | ||
| # -d/--draft or --no-draft options, which set draft mode to true or false, respectively. | ||
| draft-by-default: true |
There was a problem hiding this comment.
Hmm...I leaning towards yes, commented out by default? I personally don't use draft PRs much - I just don't assign reviewers until the PR is ready.
index.js
Outdated
| .option('-b, --base <base>', `Specify the base branch to use for the first and combined PRs.`, defaultBase) | ||
| .option('-c, --create-prs', 'Create GitHub PRs from your train branches'); | ||
| .option('-b, --base <base>', `Specify the base branch to use for the first and combined PRs.`, defaultBase); | ||
| if (draftByDefault) { |
There was a problem hiding this comment.
Do you think it's necessary to have this if here? I think it might be better to simply always show all the options (so that the user knows what they can do) - I'd say it's somewhat unusual to change what flags are allowed based on what defaults are set?
# Conflicts: # cfg_template.yml # index.js
# Conflicts: # index.js
greglockwood
force-pushed
the
feature/support-drafts
branch
from
ed49e02 to
e83af0e
Compare
Which problem does this PR solve?
I personally have found that the PRs created by
pr-trainare often not ready to be seen by others on the team yet.I regularly find myself having to add the following:
Thus, utilizing the ability to create draft PRs, which suppress notifications to code owners and others via Slack/email is handy until I have added all those details.
I would go so far as to say that I wish for the tool to always create draft PRs unless I override this behaviour.
Main changes
Added a command-line option,
--draft, to create PRs in draft mode.Added a
prs.draft-by-defaultsetting to the.pr-train.ymlconfig file to enable draft PRs to be the default. In this case, the command-line option becomes--no-draftto override the default behaviour on a per-command basis.Related documents/resources
Create a pull request GitHub REST API docs.
Suggested CR ordering of files?
cfg_template.ymlREADME.mdgithub.jsindex.jsHow I tested it works
I tested the following scenarios:
draft-by-defaultsetting in config file, no--draftcommand-line flag set. PRs created in non-draft status, maintaining the current behaviour.draft-by-default: truesetting in config file, no--draftcommand-line flag set. PRs created in draft status, which is correct.draft-by-default: truesetting in config file,--no-draftcommand-line flag set. PRs created in non-draft status, which is correct.draft-by-defaultsetting in config file,--draftcommand-line flag set. PRs created in draft status, which is correct.