Clean shell scripts #7826
Clean shell scripts #7826
Conversation
| npm publish | ||
| cd - | ||
| ) |
There was a problem hiding this comment.
This is a nice trick! Though I'm not sure if it'd be clear for everyone what is it doing. I'd stick with the cd - just for clarity.
There was a problem hiding this comment.
I can add a comment like: using a subshell to avoid having to cd back.
It's not just to avoid writing a line, it also can prevent some troubles, see SC2013.
There was a problem hiding this comment.
Oh, I didn't know about that one! It should be modified then. The comment would be helpful :)
scripts/release.sh
Outdated
| if [[ -z $RELEASE_TAG ]]; then | ||
| npm publish | ||
| else | ||
| npm publish --tag $RELEASE_TAG | ||
| npm publish --tag "$RELEASE_TAG" | ||
| fi | ||
| cd - |
There was a problem hiding this comment.
In any case, for consistency, shouldn't this cd - be replaced as well?
There was a problem hiding this comment.
Indeed. Will do. I'm not sure if shellcheck would have spotted it after running it a second time. Will try. If not, we may found a bug....
There was a problem hiding this comment.
Opened an issue. :)
Hi,
I fixed various things spotted by shellcheck.
What kind of change does this PR introduce? (check at least one)
Does this PR introduce a breaking change? (check one)
If yes, please describe the impact and migration path for existing applications:
The PR fulfills these requirements:
devbranch for v2.x (or to a previous version branch), not themasterbranchfix #xxx[,#xxx], where "xxx" is the issue number)