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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Mar 15, 2019
Merged

Documentation suggestions for the deploy, PR process #198

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e3822e4
Create PULL_REQUEST_TEMPLATE.md
Mar 13, 2019
c7442b4
add master heading to changelog
Mar 13, 2019
f0a3764
build out deploy steps
Mar 13, 2019
644be0b
fix typos
Mar 14, 2019
cf9e349
Merge branch 'master' into update-deploy-steps
Mar 15, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .github/PULL_REQUEST_TEMPLATE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<!-- Thanks for the PR! Feel free to add or remove items from the checklist. -->

- [ ] briefly describe the changes in this PR
- [ ] write tests for all new functionality
- [ ] update CHANGELOG.md with changes under `master` heading before merging
41 changes: 32 additions & 9 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

npm install & npm start & open http://localhost:9966/

You'll need a [Mapbox access token](https://www.mapbox.com/help/create-api-access-token/) stored in localstorage. Set it via
You'll need a [Mapbox access token](https://docs.mapbox.com/help/how-mapbox-works/access-tokens/) stored in localstorage. Set it via

localStorage.setItem('MapboxAccessToken', '<TOKEN HERE>');

Expand All @@ -16,13 +16,36 @@ Lastly, run the test command from the console:

npm test


## Deploying

- `npm run prepublish && npm run docs`
- Update the version key in [package.json](https://github.com/mapbox/mapbox-gl-geocoder/blob/master/package.json#L3)
- Outline changes in [CHANGELOG.md](https://github.com/mapbox/mapbox-gl-geocoder/blob/master/CHANGELOG.md)
- Commit and push
- `git tag -a vX.X.X -m 'vX.X.X'`
- `git push --tags`
- `npm publish`
- Update version number in GL JS examples ([one](https://github.com/mapbox/mapbox-gl-js/blob/publisher-production/docs/pages/example/mapbox-gl-geocoder.html), [two](https://github.com/mapbox/mapbox-gl-js/blob/publisher-production/docs/pages/example/point-from-geocoder-result.html), [three](https://github.com/mapbox/mapbox-gl-js/blob/publisher-production/docs/pages/example/mapbox-gl-geocoder-outside-the-map.html), [four](https://github.com/mapbox/mapbox-gl-js/blob/publisher-production/docs/pages/example/mapbox-gl-geocoder-limit-region.html), [five](https://github.com/mapbox/mapbox-gl-js/blob/publisher-production/docs/pages/example/mapbox-gl-geocoder-accept-coordinates.html), [six](https://github.com/mapbox/mapbox-gl-js/blob/publisher-production/docs/pages/example/mapbox-gl-geocoder-proximity-bias.html), [seven](https://github.com/mapbox/mapbox-gl-js/blob/publisher-production/docs/pages/example/forward-geocode-custom-data.html))
Follow this deploy process after all changes for the release are merged into master. You will copy and paste this checklist in the comment of the release pull request.

```
## Release checklist

1. Create a branch off `master` and a pull request with the following changes. Copy this checklist in the comment of the pull request.
- [ ] Update the [CHANGELOG.md](https://github.com/mapbox/mapbox-gl-geocoder/blob/master/CHANGELOG.md) by comparing the last release and what is on `master`. In the changelog, replace the `master` heading with the to-be-released stable version.
- [ ] Update the version number in `package.json` and `package-lock.json`.
3. Request a PR review 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`
Copy link
Collaborator

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.

Copy link
Contributor Author

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.

- [ ] 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`
Copy link
Collaborator

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.

Copy link
Contributor Author

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?



## Post-release checklist

Update version number in GL JS examples:

* [ ] [mapbox-gl-geocoder](https://github.com/mapbox/mapbox-gl-js/blob/publisher-production/docs/pages/example/mapbox-gl-geocoder.html)
* [ ] [point-from-geocoder-result](https://github.com/mapbox/mapbox-gl-js/blob/publisher-production/docs/pages/example/point-from-geocoder-result.html)
* [ ] [mapbox-gl-geocoder-outside-the-map](https://github.com/mapbox/mapbox-gl-js/blob/publisher-production/docs/pages/example/mapbox-gl-geocoder-outside-the-map.html)
* [ ] [mapbox-gl-geocoder-limit-region](https://github.com/mapbox/mapbox-gl-js/blob/publisher-production/docs/pages/example/mapbox-gl-geocoder-limit-region.html)
* [ ] [mapbox-gl-geocoder-accept-coordinates](https://github.com/mapbox/mapbox-gl-js/blob/publisher-production/docs/pages/example/mapbox-gl-geocoder-accept-coordinates.html)
* [ ] [mapbox-gl-geocoder-proximity-bias](https://github.com/mapbox/mapbox-gl-js/blob/publisher-production/docs/pages/example/mapbox-gl-geocoder-proximity-bias.html)
* [ ] [forward-geocode-custom-data](https://github.com/mapbox/mapbox-gl-js/blob/publisher-production/docs/pages/example/forward-geocode-custom-data.html)
```