Skip to content
This repository was archived by the owner on Nov 8, 2024. It is now read-only.

Remove unused code & drop related dependencies #537

Closed
wants to merge 4 commits into from

Conversation

realityking
Copy link

Working on #536 I noticed there are a few dependencies that are actually not used in production code. This PR cleans that up.

@kuba-kubula
Copy link
Member

@realityking Hi there, these lib utilities are there for providing easier backward compatibility with previous versions of gavel.js

The decision is to keep them, also the number of dependencies is not that large (actually just 2 small packages)

@realityking
Copy link
Author

@kuba-kubula I don't follow. Since gavel is bundled into a single file through rollup, the code included in the files I'm removing in this PR is never shipped as part of the npm package.

You can verify this by building the master branch and this branch and comparing the content of the package.

Size wise these dependencies are indeed tiny (3.1% of the node_modules folder size) but 4 dependencies less is IMHO still a win if they are consequence free.

@kuba-kubula
Copy link
Member

@realityking hm 🤔 you're right. I just found out that the .npmignore list contains the lib folder. 🦊
Will fix that.

@kuba-kubula
Copy link
Member

@realityking I think moving those dependencies to devDeps will be enough for your use-case/needs, correct?

@realityking
Copy link
Author

@kuba-kubula To get the dependencies out of my tree, yes, absolutely. That said, I'd love to contribute some refactorings like #536 and #538 to gavel and eliminating unused code makes that a lot easier.

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

Successfully merging this pull request may close these issues.

2 participants