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

Remove Storybook #11649

Merged
merged 1 commit into from
Dec 15, 2021
Merged

Remove Storybook #11649

merged 1 commit into from
Dec 15, 2021

Conversation

jtpio
Copy link
Member

@jtpio jtpio commented Dec 9, 2021

References

In #11648 we noticed some dependencies like nth-check are difficult to update and come from the dependency on storybook.

Storybook seems to be coming back often when we try to update dependencies, for example:

I'm not sure if anyone is currently using it? Which is why this PR proposes we just remove it. If someone is still using, then we should consider keeping it.

Code changes

User-facing changes

None

Backwards-incompatible changes

Should be none.

@jtpio jtpio added this to the 4.0 milestone Dec 9, 2021
@jupyterlab-probot
Copy link

Thanks for making a pull request to jupyterlab!
To try out this branch on binder, follow this link: Binder

@jtpio
Copy link
Member Author

jtpio commented Dec 9, 2021

cc @telamonian if you have any input on this

@fcollonval
Copy link
Member

Thanks for starting this.

We spoke briefly about that in #11173 (comment) @jasongrout and I.

The agreement was indeed to drop it as no active developers use it (this is probably the reason it was never updated 😉).

@jtpio
Copy link
Member Author

jtpio commented Dec 10, 2021

Marking as ready as I'm not planning to make any more changes.

We can also leave it open for a bit so folks have time to comment. This can also be discussed at the next weekly meeting.

@jtpio jtpio marked this pull request as ready for review December 10, 2021 07:47
@jtpio
Copy link
Member Author

jtpio commented Dec 10, 2021

Also the diff cleans up the yarn.lock quite a bit:

image

Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

Thanks @jtpio

@github-actions
Copy link
Contributor

Benchmark report

The execution time (in milliseconds) are grouped by test file, test type and browser.
For each case, the following values are computed: min <- [1st quartile - median - 3rd quartile] -> max.

The mean relative comparison is computed with 95% confidence.

Results table
Test file large_code_notebook large_md_notebook
open
chromium
actual 3747 <- [4020 - 4184 - 4237] -> 5140 2604 <- [2760 - 2840 - 2887] -> 2962
expected 3783 <- [3945 - 4152 - 4227] -> 4341 2604 <- [2736 - 2814 - 2887] -> 4761
Mean relative change 0.8% ± 1.2% -0.2% ± 1.7%
switch-from
chromium
actual 465 <- [530 - 569 - 701] -> 795 396 <- [431 - 445 - 465] -> 615
expected 471 <- [541 - 570 - 701] -> 827 395 <- [430 - 447 - 466] -> 670
Mean relative change -2.6% ± 4.2% -1.2% ± 3.2%
switch-to
chromium
actual 532 <- [583 - 618 - 635] -> 1777 439 <- [486 - 519 - 529] -> 553
expected 520 <- [585 - 618 - 638] -> 778 443 <- [487 - 519 - 530] -> 1391
Mean relative change 1.2% ± 4.2% -1.4% ± 3.6%
close
chromium
actual 483 <- [529 - 556 - 605] -> 940 387 <- [416 - 437 - 452] -> 494
expected 461 <- [543 - 579 - 845] -> 945 391 <- [414 - 434 - 451] -> 506
Mean relative change -8.8% ± 5.9% 0.4% ± 1.5%

Changes are computed with expected as reference.

@fcollonval fcollonval merged commit a783add into jupyterlab:master Dec 15, 2021
@jtpio jtpio deleted the remove-storybook branch December 15, 2021 13:56
@fcollonval
Copy link
Member

Sorry I closed it a bit too quickly. I will raise the point at today's team meeting. And revert if needed.

@jtpio jtpio mentioned this pull request Jan 4, 2022
3 tasks
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants