serverless / serverless-azure-functions

Serverless Azure Functions Plugin – Add Azure Functions support to the Serverless Framework
MIT License
266 stars 159 forks source link

fix: remove unused `semver` dependency #668

Closed G-Rath closed 1 year ago

G-Rath commented 1 year ago

What did you implement:

The semver package is no longer being used, so it doesn't need to be listed as a dependency.

How did you implement it:

I removed it from dependencies in package.json

How can we verify it:

❯ rg --files-with-matches 'semver'
integrationTests/configurations/node16-linux-webpack/package-lock.json
integrationTests/configurations/node12-linux/package-lock.json
integrationTests/configurations/node18-linux-webpack/package-lock.json
integrationTests/configurations/node14-windows/package-lock.json
integrationTests/configurations/node16-linux/package-lock.json
integrationTests/configurations/node12-linux-webpack/package-lock.json
integrationTests/configurations/node16-windows-webpack/package-lock.json
integrationTests/configurations/node14-windows-webpack/package-lock.json
integrationTests/configurations/node18-windows-webpack/package-lock.json
integrationTests/configurations/node16-windows/package-lock.json
integrationTests/configurations/node18-windows/package-lock.json
integrationTests/configurations/node12-windows-webpack/package-lock.json
integrationTests/configurations/node12-windows/package-lock.json
integrationTests/configurations/node14-linux-webpack/package-lock.json
integrationTests/configurations/node14-linux/package-lock.json
integrationTests/configurations/node18-linux/package-lock.json
examples/typescript-webpack/package-lock.json
package-lock.json

If semver was being used by this package, then there should have been a match in either a TypeScript or JavaScript file, which there was not.

The tests also will fail if semver is actually needed.

Todos:

Note: Run npm run test:ci to run all validation checks on proposed changes

Is this ready for review?: YES Is it a breaking change?: NO

G-Rath commented 1 year ago

It looks like acorn is also completely unused, and that a single lodash function is being used in a test file. It also looks like azure-functions-core-tools is unused but actually it's been called via spawn.

@gligorkot ~I'm happy to do PRs removing acorn and at least marking lodash as a dev dependency if not removing it completely.~ I've opened #669 and #670 too; these will possibly conflict with each other as they're merged, but I'm happy to handle rebasing them.

gligorkot commented 1 year ago

Sounds good @G-Rath, I'll have a look at these a bit later today. It would be great if we can eliminate any unused dependency and then upgrade all of the dependencies and write tests for all the functionality. Then we could release a v3.0.0 which would be a major step forward to keeping this plugin up to date with the latest web technologies

G-Rath commented 1 year ago

@gligorkot awesome - fully support doing some dependency upgrades, this plugin is a bit behind the times 😅

I've had a look over the dependencies and have a few suggestions if you'd like them.

gligorkot commented 1 year ago

@G-Rath please! Unfortunately, I've got a bit on over the next few weeks at work so probably won't put in any time into the upgrades, but feel free to start a branch and I can join in and contribute as well when able to.

Also, if you can add to the tests as we're upgrading things that would be great too.

G-Rath commented 1 year ago

@gligorkot unfortunately I'm pretty busy myself right now, but I'll see what I can do. If we can get a couple of the prs I've opened so far that'd help because then I won't need approval for the CI workflows to run.

I'll post a write-up of my thoughts on each dependency sometime this week