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.

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

ci: test storybook@next #2496

Merged
merged 2 commits into from
Feb 22, 2021
Merged

ci: test storybook@next #2496

merged 2 commits into from
Feb 22, 2021

Conversation

merceyz
Copy link
Member

@merceyz merceyz commented Feb 21, 2021

What's the problem this PR addresses?

The current latest release of storybook (6.1.18) has an undeclared dependency that we could have caught earlier

cc @gaetanmaisse

How did you fix it?

Test the next version of storybook

Checklist

  • I have read the Contributing Guide.
  • I have set the packages that need to be released for my changes to be effective.
  • I will check that all automated PR checks pass before the PR gets reviewed.

@merceyz merceyz requested a review from arcanis as a code owner February 21, 2021 14:11
@merceyz
Copy link
Member Author

merceyz commented Feb 21, 2021

The stuff failing now was introduced in storybookjs/storybook#13808 cc @shilman

@shilman
Copy link
Contributor

shilman commented Feb 21, 2021

@merceyz sorry about that. @gaetanmaisse and I are on it--thanks for your patience! 🙏

@gaetanmaisse
Copy link
Contributor

gaetanmaisse commented Feb 21, 2021

I fixed the error currently throwing on master a few days ago (storybookjs/storybook#13916) and thought @shilman cherry-picked it to master but it looks like there was a misunderstanding, so I made a PR to fix master (storybookjs/storybook#13991), I think it will be merged and released in the next 24h.

I don't really know what to think about using next in your E2E as this branch can be quite unstable... I don't want to give you some extra work/failing runs... we have the same E2E test on our side and we clearly neglected it for the last PRs merged on master, it's on us 😞 .

Edit: And this one is to fix next storybookjs/storybook#13992

@merceyz
Copy link
Member Author

merceyz commented Feb 22, 2021

I don't really know what to think about using next in your E2E as this branch can be quite unstable... I don't want to give you some extra work/failing runs

That's fine and kind of the point of the e2e test, to catch issues as soon as possible, besides our CI has been red for months so nothing new there. (last time it was green: e448b9e)

@arcanis arcanis merged commit ff69c55 into master Feb 22, 2021
@arcanis arcanis deleted the merceyz/storybook-e2e branch February 22, 2021 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants