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

chore: remove assert dependencies #26276

Closed
wants to merge 1 commit into from
Closed

Conversation

43081j
Copy link
Contributor

@43081j 43081j commented Mar 1, 2024

This removes the assert dependency from docs-tools since it seems it is no longer used.

If for some reason we do actually need it, i suggest we at least replace it with a smaller alternative as assert is a known-to-be-bloated dependency

This removes the `assert` dependency from docs-tools since it seems it
is no longer used.
@@ -48,7 +48,6 @@
"@storybook/preview-api": "workspace:*",
"@storybook/types": "workspace:*",
"@types/doctrine": "^0.0.3",
"assert": "^2.1.0",
Copy link
Member

Choose a reason for hiding this comment

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

Hey @shilman Could you make sure this is actually not needed? Thanks!

@yannbf yannbf added maintenance User-facing maintenance tasks ci:normal addon: docs labels Mar 4, 2024
@shilman
Copy link
Member

shilman commented Mar 4, 2024

Hi @43081j ! Thanks for trying to clean this up. We added assert as a dependency because it's used by doctrine, which is also a dependency of docs-tools. We should remove doctrine from docs-tools because it's unmaintained. But that is a bigger project. Closing this PR for now but please let me know if you think I got it wrong.

@shilman shilman closed this Mar 4, 2024
@43081j
Copy link
Contributor Author

43081j commented Mar 4, 2024

@shilman that sounds like your dependency tree isn't right?

docs-tools doesn't reference assert anywhere from what i can see. so are you saying doctrine depends on it but doesn't declare it as a dependency?

we should figure this out, it seems one way or another it needs removing from the dependencies (thanks to not being used directly)

@shilman
Copy link
Member

shilman commented Mar 4, 2024

@43081j absolutely. here is the background for the change that added assert #24732

if we can come up with a good replacement for doctrine i'd love to get rid of it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addon: docs ci:normal maintenance User-facing maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants