-
Notifications
You must be signed in to change notification settings - Fork 76
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 support for npm publish --dry-run #8
Conversation
Add an extra Option/command line argument to optionally run npm publish with the --dry-run flag enabled to prevent actual publishing. Intended for use in testing workflows. This option/argument defaults to false.
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.
Thanks for the PR, and especially for writing so many great tests! I really appreciate that.
Everything looks good. I requested one change, and then I look forward to merging the PR. Great job!
src/npm-publish.ts
Outdated
@@ -25,7 +25,7 @@ export async function npmPublish(opts: Options = {}): Promise<Results> { | |||
|
|||
let results: Results = { | |||
package: manifest.name, | |||
type: diff || "none", | |||
type: options.dryRun ? "dry-run" : (diff || "none"), |
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.
Instead of overloading the type
property, let's add a new property to the Results
object (i.e. dryRun: boolean
). That way, scripts can still check the type
property like normal, regardless of whether it was a dry run or not.
debug: debugHandler, | ||
}; | ||
|
||
// Puglish to NPM | ||
// Publish to NPM |
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.
Thank you for fixing my typos. 🤦♂️
@@ -25,7 +25,7 @@ export const npm = { | |||
|
|||
options.debug(`Running command: npm view ${name} version`); | |||
|
|||
// Run NPM to get the latest published versiono of the package | |||
// Run NPM to get the latest published version of the package |
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.
Thank you for fixing my typos. 🤦♂️
@JamesMessinger thanks for the feedback. |
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.
Thanks for making the changes. Looks great!
From #6: Add an extra Option/command line argument to optionally run npm publish with the --dry-run flag enabled to prevent
actual publishing. Intended for use in testing workflows. This option/argument defaults to false.
Wasn't sure if build files should be committed, so I left them out for now.