-
Notifications
You must be signed in to change notification settings - Fork 181
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
Documentation suggestions for the deploy, PR process #198
Conversation
- [ ] Commit and push with commit message `vX.X.X` | ||
- [ ] Create the git tag for the release with `git tag -a vX.X.X -m 'vX.X.X'` | ||
- [ ] Push the tags with `git push --tags` | ||
- [ ] Run `npm publish` |
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've been bitten in the past with non-committed files containing secret keys in the same directory, which to much horror npm publish
silently packaged up and included in the deploy 😱. In that regard I find it safer to use CI to publish the release, that said so long as people doing the npm publish are aware of this then it should be okay.
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.
find it safer to use CI to publish the release
Oh, smart! I'm not familiar with this, what does that look like in practice?
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.
This is great @katydecorah, I think this extra rigour and documentation to the process improves the quality of the project. Apart from the comments I've made, it looks good to me.
3. Request a review PR and then merge it into `master`. | ||
4. Tag the release and start the build. | ||
- [ ] Make sure you've pulled in all changes to `master` locally. | ||
- [ ] Build the release with `npm run prepublish && npm run docs` |
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.
Do you think doc changes should be committed as part of each PR in the lead up to a release, or left out to the release PR?
I think it's better to commit them as we go for each feature, that way you have complete docs for any given commit.
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 think it's better to commit them as we go for each feature, that way you have complete docs for any given commit.
I agree, I think we should encourage PRs to update the changelog before merging to help keep track of what's in master. I included this as a step in the pull request template.
* master: prepare to publish v3.1.6 update package-lock bump package version to v3.1.5 update changelog for v3.1.5 use indexOf instead of includes use XMLHttpRequest use sha.js instead of crypto use https library instead of request
* version4: (25 commits) fix focus trap (#220) add `npm run docs` step to template (#221) Fix typo in documentation additional properties existance check Ensure properties exist prior to checking id update changelog remove use of hardcoded feature IDs update outdated packages with npm update update changelog Bump insecure dev dependencies tests for options.flyTo Update changelog update jsdoc and api docs pass flyTo options to map upon selection extend eslint, add precommit hook (#210) replace sha.js with nanoid reduce bundle size to 42kb (13kb gzipped) [docs] update deploy process (#198) prepare to publish v3.1.6 update package-lock bump package version to v3.1.5 ...
I'd like to propose some documentation updates to the deploy process. The goal is to help make it clear for contributors what to include in their PR and how to deploy. In this PR, I propose:
master
heading.@andrewharvey I'd love to hear your feedback on these proposed updates 🙏