Skip to content
This repository has been archived by the owner on Jan 13, 2024. It is now read-only.

chore: remove unused dependencies #1769

Merged
merged 4 commits into from
Mar 28, 2023
Merged

chore: remove unused dependencies #1769

merged 4 commits into from
Mar 28, 2023

Conversation

ignatiusmb
Copy link
Contributor

See if this works, we might be able to remove a lot of dependencies that's only used (require-d) in tests

@ignatiusmb
Copy link
Contributor Author

Seems like it's working, pushing one more change and this should be ready

@ignatiusmb ignatiusmb marked this pull request as ready for review September 23, 2022 05:56
jesec
jesec previously requested changes Sep 25, 2022
@@ -60,14 +59,6 @@
"simple-git-hooks": ">=2.8.0",
"typescript": "4.7.2"
},
"peerDependencies": {
"node-notifier": ">=9.0.1"
Copy link
Contributor

Choose a reason for hiding this comment

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

We do have dependency on "node-notifier" in "test-79-npm" at the moment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's what I was trying to see in the first commit, and all the tests seems to pass.

It confirms my suspicion with the "peerDependenciesMeta" that this package won't be installed unless specified explicitly, so it's not installed even before this change, which we can confirm by looking at the lockfile not having "node-notifier" anywhere.

Another thing is that "test-79-npm" seems to download all the packages by itself using https module, which might not be the best approach to testing these kinds of stuff, but that would also mean we may not need any of those packages in the project's package.json.

@github-actions
Copy link

This pull-request is stale because it has been open 90 days with no activity. Remove the stale label or comment or this will be closed in 5 days. To ignore this pull-request entirely you can add the no-stale label

@github-actions github-actions bot added the Stale label Feb 27, 2023
@ignatiusmb
Copy link
Contributor Author

What's left to do?

@github-actions github-actions bot removed the Stale label Feb 28, 2023
@baparham baparham dismissed jesec’s stale review March 27, 2023 08:06

issue seems addressed, maintainer no longer actively involved in project

@baparham
Copy link
Contributor

I think it's reasonable to dismiss Jesse's review since he's no longer actively involved, and his original concern does seem to be addressed to me. I'll go ahead and dismiss the 'changes requested' flag, and document my justification here.

@baparham
Copy link
Contributor

@ignatiusmb perhaps a rebase and rerun of the tests and we'll see how that goes

@robertsLando robertsLando merged commit c353cc9 into vercel:main Mar 28, 2023
@ignatiusmb ignatiusmb deleted the unused branch March 28, 2023 07:24
pcnate pushed a commit to pcnate/pkg that referenced this pull request Aug 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants