-
Notifications
You must be signed in to change notification settings - Fork 146
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
build(dev-deps): consolidate all common dev deps in workspaces #605
Conversation
packages/commons/package.json
Outdated
@@ -59,4 +44,4 @@ | |||
"dependencies": { | |||
"@types/aws-lambda": "^8.10.72" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be moved as a dev-dependency too right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to check this one, I assumed it had to stay but maybe it can be removed. I'll test and get back to you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok I've tested this and you were right, I removed it from this file and added it to the main package.json
as devDependency
in the last commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's great to hear, and it means the library's prod bundle decreased :)
"@types/aws-lambda": "^8.10.72" | ||
} | ||
} | ||
"dependencies": {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Nicely done 🥳
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description of your changes
In continuing the effort to reduce redundant code in the repo, and after the consolidation of local & CI environments around
[email protected]
and subsequent full adoption ofworkspaces
; this PR pulls-up all the shared dev dependencies from the package-specificpackage.json
files to the one in the root.With the previous PRs the dependencies were being already installed in the main
node_modules
directory so this change only affects their placement. This will also help in the long term to keep more consistent versioning across shareddevDependencies
and also reduce Dependabot's diff sizes.How to verify this change
See successful checks under this PR and the e2e successful execution for this branch.
Related issues, RFCs
#484
PR status
Is this ready for review?: YES
Is it a breaking change?: NO
Checklist
Breaking change checklist
N/A
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.