-
Notifications
You must be signed in to change notification settings - Fork 61
DOCSP-4862 Add script to update browser sdk README versions #254
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
Conversation
adamchel
left a comment
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.
Looks good, except that the changes need to get committed before a publish. should be good after this but I wanna take another quick look then
|
|
||
| ```bash | ||
| lerna publish --force-publish="*" | ||
| ./update_browser_readme_version.sh |
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.
[minor] this should go before lerna publish, otherwise the READMEs on npmjs.com will show an old version.
| sed -i '' \ | ||
| -e "s#\(stitch-sdks/js/bundles/\)[0-9\.]*\(/stitch\)#\1$VERSION_MAJOR.$VERSION_MINOR.$VERSION_PATCH\2#g" \ | ||
| ../packages/browser/sdk/README.md | ||
| echo "README updated." |
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.
[minor] To make sure this gets committed before a publish you should add
git add ../packages/browser/sdk/README.md
git commit -m "Update SDK version to $VERSION_MAJOR.$VERSION_MINOR.$VERSION_PATCH in relevant READMEs"
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.
I'm not sure how I feel about adding git commands in a script... Wouldn't this violate the principle of least astonishment? I wouldn't expect a script to make a commit on my behalf.
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.
I put the reminder to commit in the contrib/README.md. Thoughts?
Please run contrib/update_browser_readme_version.sh when releasing/publishing docs.
d1da23a to
0f18a39
Compare
0f18a39 to
e77c88f
Compare
| fi | ||
|
|
||
| echo "README updated." | ||
| git add -u "../packages/browser/sdk/README.md" |
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.
[q] why git add -u? we don't want to add all changes
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.
-u is just update which means it won't add that file if it wasn't already tracked. Not really necessary I suppose but it doesn't hurt either.
- Note in README that you should not do this on a dirty repo (though it will fail out anyway)
1ca3847 to
3a2ae7d
Compare
adamchel
left a comment
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.
LGTM
Please run contrib/update_browser_readme_version.sh when releasing/publishing
docs.