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.

Sign up for GitHub

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

Jump to bottom

chore(dependencies): remove unused package #317

Merged
merged 1 commit into from
Oct 13, 2020
Merged

chore(dependencies): remove unused package #317

merged 1 commit into from
Oct 13, 2020

Conversation

ismay
Copy link
Member

@ismay ismay commented Oct 13, 2020

Closes #312

I don't see this package being imported anywhere. The build and storybook also still work after removing. Yarn why shows this:

Screenshot 2020-10-13 at 15 15 55

I think it's safe to remove it. Seems like it's bundled in the app-shell.

@ismay ismay requested a review from a team as a code owner October 13, 2020 13:25
@cypress
Copy link

cypress bot commented Oct 13, 2020 •
edited
Loading


Test summary

494 • 0 • 0 • 0

Run details
Project ui
Status Passed
Commit e87a945
Started Oct 13, 2020 3:05 PM
Ended Oct 13, 2020 3:17 PM
Duration 11:48 💡
OS Linux Ubuntu Linux - 18.04
Browser Electron 80

View run in Cypress Dashboard ➡️

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@Mohammer5
Copy link
Contributor

Hmm that's odd. I think the reason for this package is to have it in the storybook

@amcgee
Copy link
Member

amcgee commented Oct 13, 2020

@ismay I think it's needed here (for storybook, as @Mohammer5 said) - https://github.com/dhis2/ui/blob/master/storybook/.storybook/preview.js#L1 - so I don't think we can remove it

amcgee
amcgee requested changes Oct 13, 2020
View reviewed changes
Copy link
Member

@amcgee amcgee left a comment

Choose a reason for hiding this comment

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

Pretty sure we have to keep this...

@ismay
Copy link
Member Author

ismay commented Oct 13, 2020 •
edited
Loading

Ah. rg (which I used to search for mentions of roboto in the project) must have ignored the storybook dir. So but we don't need it in the root package then do we? I don't see any imports outside of storybook.

@ismay
Copy link
Member Author

ismay commented Oct 13, 2020

I think the root one is just a leftover from what Viktor mentioned: https://dhis2.slack.com/archives/CBM8LNEQM/p1602598734035200

@ismay ismay requested a review from amcgee October 13, 2020 14:58
amcgee
amcgee approved these changes Oct 13, 2020
View reviewed changes
Copy link
Member

@amcgee amcgee left a comment

Choose a reason for hiding this comment

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

Yep, think so 👍

@ismay ismay merged commit bc32186 into master Oct 13, 2020
@ismay ismay deleted the remove-typeface-roboto branch October 13, 2020 15:42
@dhis2-bot
Copy link
Contributor

🎉 This PR is included in version 5.7.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amcgee amcgee amcgee approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ismay @Mohammer5 @amcgee @dhis2-bot