search

apiaryio / gavel.js

Validator of HTTP messages (JavaScript implementation)
MIT License
97 stars 22 forks source link

Remove unused code & drop related dependencies #537

Closed realityking closed 3 years ago

realityking commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

@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 commented 3 years ago

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

kuba-kubula commented 3 years ago

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

realityking commented 3 years ago

@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.