Skip to content
This repository has been archived by the owner on Feb 27, 2020. It is now read-only.

Update react-storybook to latest version to fix iframe on docs site #154

Merged
merged 2 commits into from
Apr 26, 2017

Conversation

halkeye
Copy link
Member

@halkeye halkeye commented Feb 27, 2017

Description

Submitter checklist

  • Link to JIRA ticket in description, if appropriate.
  • Change is code complete and matches issue description
  • Appropriate unit or acceptance tests or explanation to why this change has no tests

Reviewer checklist

  • Run the changes and verified the change matches the issue description
  • Reviewed the code
  • Verified that the appropriate tests have been written or valid explanation given

@michaelneale
Copy link
Member

cc @sophistifunk @cliffmeyers look ok?

@sophistifunk
Copy link
Collaborator

Does this build in our cloud? Github is telling me it failed, but the URL is bad.

@michaelneale
Copy link
Member

@sophistifunk yeah it does - but a test instance (noting to do with dogfood) wallopped the status.

@dragoonis if you push a change it shoudl trigger another build.

@michaelneale
Copy link
Member

@dragoonis you have an invitation to be a collaborator on this repo so you can work in branches here too if that suits.

@cliffmeyers
Copy link
Collaborator

@michaelneale this PR doesn't show up on the PR's tab on Dogfood. Should it? Otherwise I'm not sure how to re-run the build.

@michaelneale
Copy link
Member

I don't know either - convert it to a branch and it should rebuild.

@halkeye
Copy link
Member Author

halkeye commented Mar 14, 2017

@sophistifunk
Copy link
Collaborator

I think https://github.com/jenkinsci/jenkins-design-language/blob/master/site-publish.sh isn't part of the build anymore.

Never was, as it doesn't work for everybody.

scherler pushed a commit that referenced this pull request Apr 1, 2017
UX-314: change animation timing
@scherler
Copy link
Collaborator

@sophistifunk what is the problem. I fixed it around a month ago when i published the last time

@sophistifunk
Copy link
Collaborator

IIRC, it required a specific version of git that's newer than what you get on OSX.

@scherler
Copy link
Collaborator

@sophistifunk yeah I added:

git merge --no-edit -s ours remotes/webpages/gh-pages --allow-unrelated-histories

--allow-unrelated-histories is to allow that merge if you are on git 2.9+

package.json Outdated
@@ -47,7 +47,7 @@
"react-dom": "15.3.2"
},
"devDependencies": {
"@kadira/storybook": "2.24.1",
"@kadira/storybook": "^2.35.3",
Copy link
Collaborator

Choose a reason for hiding this comment

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

"@kadira/storybook": "2.35.3" please do not use ^

Copy link
Collaborator

Choose a reason for hiding this comment

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

I was thinking, should we write a utility that validates package.json for semver best practices. My feeling is that deps and devDeps should use exact, but peerDeps should use caret operator. We could create a standalone util for use in JDL but also bake it into js-builder by default. Or maybe something comparable already exists, can look. WDYT @scherler ?

@halkeye
Copy link
Member Author

halkeye commented Apr 20, 2017

Doesn't yarn.lock handle that anyways? I can't fix this or on mobile so I'll try and get to it asap

@michaelneale michaelneale dismissed scherler’s stale review April 21, 2017 00:35

No longer uses caret

@michaelneale
Copy link
Member

@halkeye yarn isn't in use yet (and probably better to be explicit, like you have been).

@scherler package.json has been updated.

@michaelneale
Copy link
Member

ping @scherler @sophistifunk can we merge this one in now?

@sophistifunk
Copy link
Collaborator

Definitely

@michaelneale michaelneale merged commit 35758c1 into jenkinsci:master Apr 26, 2017
@michaelneale
Copy link
Member

nice one @halkeye !

swish

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants