Improve publish script #570
Conversation
also fix cleaning previous internals directory
pradeepnschrodinger
changed the title
pradeepnschrodinger
requested review from
wcjordan,
somavara,
deshpsuy,
mycrobe and
novakps
| @@ -5,12 +5,12 @@ | |||
| var glob = require('glob'); | |||
| var path = require('path'); | |||
| var fs = require('fs'); | |||
| var rimraf = require('rimraf'); | |||
There was a problem hiding this comment.
Could we have legal ticket?
| set -e | ||
|
|
||
| # Defaults that can be overridden in the CLI (except the cookie file) | ||
| export BETA=0 | ||
| export BETA=false |
There was a problem hiding this comment.
Let's make this a boolean flag and avoid passing a value for the flag.
i.e, instead of --beta 1 we can simply use --beta
| set -e | ||
|
|
||
| # Defaults that can be overridden in the CLI (except the cookie file) | ||
| export BETA=0 | ||
| export BETA=false | ||
| export LATEST=false |
There was a problem hiding this comment.
If LATEST is true, the script will update and publish the latest tag.
build_helpers/publishPackage.sh
Outdated
| * ) # Invalid flag | ||
| echo "Invalid flag: $opt" | ||
| exit 192 | ||
| ;; | ||
| esac | ||
| done | ||
|
|
||
| current_version=$(node -p "require('./package').version") | ||
| # branch name is required |
There was a problem hiding this comment.
Earlier we always pushed under master.
Now that we might run the script from multiple branches like v1.0.x or v.1.1.x, let's have the user specify the upstream branch.
build_helpers/publishPackage.sh
Outdated
| exit 1 | ||
| fi | ||
| # prompt next version | ||
| while [ true ]; do |
There was a problem hiding this comment.
Re-prompt if user enters invalid version.
There was a problem hiding this comment.
I think the current behaviour is better. May be we can at max provide couple of more attempts in case of incorrect input but not an infinite retry.
| npm publish | ||
|
|
||
| echo "Publishing latest tag" | ||
| npm publish --tag latest |
There was a problem hiding this comment.
Not passing --tag defaults it to latest. But I feel passing it explicitly is clearer.
build_helpers/publishPackage.sh
Outdated
| @@ -1,35 +1,70 @@ | |||
| #!/bin/bash | |||
| # example usages: | |||
| # ./publishPackage.sh --branch=v1.0.x | |||
| # ./publishPackage.sh --branch=v1.1.x --latest | |||
There was a problem hiding this comment.
To run the script through npm, user will have to do:
npm run publish-package -- --branch v1.1.x --latest
The -- immediately after the script name is used for separating the params passed to the npm command itself with the params passed to publishPackage.sh
|
Great, thank you @somavara ! |
build_helpers/publishPackage.sh
Outdated
| --branch) | ||
| if [ -z $VALUE ] ; then | ||
| echo "Please specify branch name" | ||
| exit 192 |
There was a problem hiding this comment.
Could you review the exit code?
| if (!fs.existsSync(internalPath)) { | ||
| fs.mkdirSync(internalPath); | ||
| } | ||
| rimraf.sync(internalPath); // remove older entries |
There was a problem hiding this comment.
Could you check why the file extension ".sh" instead of ".js"?
We make use of a shell script (publishPackage.sh) for publishing fixed data table to npm.
The script also bumps up the package version and releases new tags prior to publishing.
I saw a few problems while working on them:
Currently, the script always overwrites the
latesttag. This'll mean that attempting to publish a new release under v1.0.x using thescript will lead to the latest tag being overwritten. This should be forbidden since we have v1.1.x.
Let's fix this by having an option flag passed to the script that overwrites the latest.
Entering the next package version exits the script if the version is invalid. Let's re-prompt the user instead.
The script does multiple operations that might require authentication. If user fails at authentication, the script just exits, and it becomes difficult to know at which step it had failed. To keep track of it, let's log before such operations.
The script for building internal doesn't clear up the internal directory if it already exists. This means, it's possible for the user to publish stale files which might have came up from a previous build, but is no longer needed.