Skip to content
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

feat(pre-publish script): Add pre-publish shell script to support standard-version prerelease #57

Merged
merged 16 commits into from
Dec 10, 2018

Conversation

iamgarima
Copy link
Contributor

@iamgarima iamgarima commented Dec 7, 2018

#58

@iamgarima iamgarima changed the title feat(pre-publish script): Add pre-publish script to support standard-version prerelease feat(pre-publish script): Add pre-publish shell script to support standard-version prerelease Dec 7, 2018
@@ -0,0 +1,17 @@
#!/usr/bin/env bash
Copy link
Member

Choose a reason for hiding this comment

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

Bash -e to abort on error

#!/usr/bin/env bash

npm install
npm test
Copy link
Member

Choose a reason for hiding this comment

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

Please remove npm test from this. Let that be in pre release only

fi

unset BRANCH
git push --follow-tags origin
Copy link
Member

Choose a reason for hiding this comment

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

Mention which branch you push to

Copy link
Member

Choose a reason for hiding this comment

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

And please put this file in /bin

Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't want the dev-time scripts to be part of the bundled package. So, we can put it in the bin folder and ensure that these files are not bundled into the tarball.

@@ -0,0 +1,15 @@
#!/bin/bash -e
Copy link
Contributor

Choose a reason for hiding this comment

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

generally, use #!/usr/bin/env sh and set -o nounset -o pipefail -o errexit . And prefer not to use bash specific syntax in the scripts. I don't see any bash specific syntax, so thats good. Why sh and not bash, cuz bash is not guaranteed to always be there. sh represents whatever is the default shell for the user running the script. I prefer the explicit set commands as opposed to the -e commandline option for bash.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not a blocker, just mentioning it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, will read on this and update the script.

@iamgarima iamgarima merged commit b6ba360 into master Dec 10, 2018
@delete-merged-branch delete-merged-branch bot deleted the standard-version-script branch December 10, 2018 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants