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

refactor: move scripts to root and use pnpm #200

Merged
merged 1 commit into from Apr 7, 2023
Merged

Conversation

raisedadead
Copy link
Member

@raisedadead raisedadead commented Apr 6, 2023

This PR moves the script packages into the repo root and uses pnpm. In a later PR, we can consolidate some of these, like chat and chat-emails and, of course, anything else that is broken.

In theory, nothing should have because they were never interdependent.

@socket-security
Copy link

socket-security bot commented Apr 6, 2023

New dependency changes detected. Learn more about Socket for GitHub ↗︎


👍 No new dependency issues detected in pull request

Bot Commands

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of package-name@version specifiers. e.g. @SocketSecurity ignore foo@1.0.0 bar@* or ignore all packages with @SocketSecurity ignore-all

Pull request alert summary
Issue Status
Install scripts ✅ 0 issues
Native code ✅ 0 issues
Bin script shell injection ✅ 0 issues
Unresolved require ✅ 0 issues
Invalid package.json ✅ 0 issues
HTTP dependency ✅ 0 issues
Git dependency ✅ 0 issues
Potential typo squat ✅ 0 issues
Known Malware ✅ 0 issues
Telemetry ✅ 0 issues
Protestware/Troll package ✅ 0 issues

📊 Modified Dependency Overview:

➕ Added Package Capability Access +/- Transitive Count Publisher
node-fetch@2.6.1 network +0 akepinski
glob@7.1.6 filesystem +1 isaacs
node-fetch@2.6.7 network +0 endless
node-fetch@2.6.6 network +0 endless
@octokit/rest@18.5.2 None +5 octokitbot
@types/node-fetch@2.6.1 None +6 types
ora@5.1.0 None +3 sindresorhus
@supercharge/promise-pool@1.7.0 None +0 marcuspoehls
dotenv@16.0.1 None +0 motdotla
dotenv@16.0.0 None +0 motdotla
winston@3.3.3 None +5 dabh
inquirer@8.2.0 network +12 sboudrias
@tryghost/content-api@1.5.16 environment +1 daniellockyer
mongodb@4.4.0 eval, network, filesystem, environment +16 dariakp

🚮 Removed packages: @octokit/rest@18.12.0, @supercharge/promise-pool@1.9.0, @tryghost/content-api@1.11.5, @types/node-fetch@2.6.2, @typescript-eslint/eslint-plugin@5.48.0, @typescript-eslint/parser@5.48.0, chalk@4.1.0, glob@7.2.3, inquirer@8.2.5, mongodb@4.6.0, winston@3.8.2

Copy link
Member

@ShaunSHamilton ShaunSHamilton left a comment

Choose a reason for hiding this comment

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

Single borken point I could find. Seems unrelated, but is still braking due to workspaces.

Which brings me to: Do we really want all of these unrelated scripts to be a part of a workspace?

To be honest, I just dislike the need to install 100s of unrelated root npm packages just because I wanted to run a single script with 2 root deps.

tricky-challenges/package.json Show resolved Hide resolved
@raisedadead
Copy link
Member Author

Agreed. I will update this in the morning.

@ojeytonwilliams
Copy link
Contributor

ojeytonwilliams commented Apr 7, 2023

Given that it takes all of 3 seconds to install everything, I'd just go with workspaces. It seems simpler to me.

install 100s of unrelated root npm packages just because I wanted to run a single script with 2 root deps.

Even that shouldn't be a problem if pnpm/pnpm#6300 is fixed.

Edit: I know this is bikeshedding territory, though, so please just do what seems reasonable. I don't want to block this.

Copy link
Member

@ShaunSHamilton ShaunSHamilton left a comment

Choose a reason for hiding this comment

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

I am happy with this 👍

@raisedadead raisedadead merged commit 4679223 into main Apr 7, 2023
2 checks passed
@raisedadead raisedadead deleted the restructure branch April 7, 2023 11:09
@raisedadead
Copy link
Member Author

Merging because I want to add a few scripts I have lying around.

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

Successfully merging this pull request may close these issues.

None yet

3 participants