Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 5 additions & 5 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ Please fill out the information below to expedite the review and (hopefully)
merge of your pull request!
-->

<!-- What changes are being made? (What feature/bug is being fixed here?) -->
<!-- What changes are being made? (What issue is being addressed here?) -->
**What**:

<!-- Why are these changes necessary? -->
**Why**:
<!-- Please provide a link to your fork's Storybook. See README notes on how to do this. -->
**Link to Storybook**:

<!-- How were these changes implemented? -->
**How**:
<!-- Are there any upstream issues or separate issues you need to reference? -->
**Additional issues**:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@priley86 priley86 Dec 8, 2017 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 👍🏼



<!-- feel free to add additional comments -->
8 changes: 4 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,11 @@ For example, to deploy to your `upstream` remote:
```
npm run storybook:deploy -- --remote=upstream
```
To deploy Storybook to a target branch and serve with rawgit instead of gh-pages, pass `--branch` flag
to `npm run storybook:deploy`.
For example, to deploy to `feature-branch` target:
To deploy Storybook to a target branch and serve with rawgit instead of gh-pages, pass `--branch` flag to `npm run storybook:deploy`. This will create a new branch to serve your Storybook (and will be useful if you have multiple open pull requests).

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect 🎉

```

## Meeting Notes
Expand Down