docs(readme): add storybook-deploy notes as well as new pr template u… - #101
Conversation
danseethaler
left a comment
danseethaler
left a comment
There was a problem hiding this comment.
Looking great! Added a couple small comments. Thanks for updating.
| --> | ||
|
|
||
| <!-- What changes are being made? (What feature/bug is being fixed here?) --> | ||
| <!-- What changes are being made? (What feature/bug is being addresses here?) --> |
| <!-- How were these changes implemented? --> | ||
| **How**: | ||
| <!-- Are there any upstream issues or separate issues you need to reference? --> | ||
| **Additional issues**: |
There was a problem hiding this comment.
I'm wondering if it makes sense to add this in addition to the issues referenced from the npm run commit process that identifies them. Will this create duplicate referencing? It may be fine, just a thought.
There was a problem hiding this comment.
@danseethaler good question... My understanding is that adding "Closes # X" within npm run commit OR adding "Closes # X" in the PR notes will close the issue. I guess my preference would be to include "Closes" in the PR comments (under the first question) instead of the commit b/c there are certain cases we might not close an issue, but still merge a commit (so you'd have flexibility to change it at any point). I'm flexible on this though (and think either is OK). Thoughts?
I made a note about additional issues at the end for a few things that you might list:
- other upstream issues / PRs affected by this PR (or related to this PR)
- other relevant issues that are related to this PR (but not closed by this PR)
I'm quite flexible on this and curious what others think...
There was a problem hiding this comment.
Yeah, it's also my preference to have the issues reference in the PR description rather than the commit message since it's much more visible when glancing at a PR. I'm mostly wondering if we're asking for it in both places if it will show multiple references in the issue we're closing. Not a huge deal either way.
I think what you've got here is good and we see how it goes 👍🏼
| For example, say you have `feature-branch`, you can deploy the storybook to a rawgit branch with: | ||
| ``` | ||
| npm run storybook:deploy -- --branch=feature-branch | ||
| npm run storybook:deploy -- --branch=feature-branch-storybook |
|
@danseethaler @jgiardino I'm going to go ahead and merge this PR as to not confuse any new contributors. I believe the Pull Request template will involve some further discussions so I have opened up #105 to address it there. |
3 participants
What:
This change updates the README as well as the PR template for notes on storybook-deploy.