New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add --allow-branch option to restrict publish to designated branches #1026
Conversation
whoops, sorry, the integration tests seem to be borked :/ |
I guess jestjs/jest#3584 is what is causing it, right? So I will rollback the jest version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove the jest bump
package.json
Outdated
@@ -81,7 +81,7 @@ | |||
"eslint-plugin-flowtype": "^2.35.0", | |||
"eslint-plugin-node": "^5.1.1", | |||
"file-url": "^2.0.2", | |||
"jest": "^19.0.2", | |||
"jest": "^21.1.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, yeah, please don't change this? It's being held back for a reason (I haven't had time to figure out why jest 20+ breaks)
basically, yeah. I haven't had time to knit the integration tests back into the regular tests (as well as make them take less time) |
README.md
Outdated
} | ||
``` | ||
|
||
and you are not on the branch `my-new-feature` currently lerna will prevent you from publishing. To force a publish anyways you can use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should read
and you are not on the branch `master`, lerna will prevent you from publishing.
To force a publish despite this config, pass the `--allow-branch` flag:
and so on
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 fixed
src/commands/PublishCommand.js
Outdated
@@ -206,6 +214,12 @@ export default class PublishCommand extends Command { | |||
throw new Error("Detached git HEAD, please checkout a branch to publish changes."); | |||
} | |||
|
|||
const currentBranch = GitUtilities.getCurrentBranch(this.execOpts); | |||
if (this.options.allowBranch && !minimatch(currentBranch, this.options.allowBranch)) { | |||
throw new Error('Branch ' + currentBranch + ' is not allowed to be published. ' + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be able to use dedent
with multi-line template strings, here.
@evocateur could you take a look again? I hope I fixed everything :) |
Looks great! |
This thread has been automatically locked because there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Description
As suggested in #1007 this introduces an
allow-branch
flag to thepublish
command.Motivation and Context
As above, I would like to use that feature too.
How Has This Been Tested?
Test suite & manual in another project
Types of changes
Checklist: