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

chore(scripts): remove devDeps defined in monorepo root #3138

Merged

Conversation

trivikr
Copy link
Member

@trivikr trivikr commented Jan 5, 2022

Issue

Related to using root tsconfig for all clients in aws-sdk-js-v3
Refs: #1307

Description

Removes devDeps defined in monorepo root from clients.
Individual devDeps, if present, were removed in #3139

Testing

Testing that the packages work without dependencies is done in #3139

Verified that dependencies are not added during client generation with client-acm:

$ sed -n 53,58p clients/client-acm/package.json 
  },
  "devDependencies": {
    "@aws-sdk/service-client-documentation-generator": "3.38.0",
    "@types/node": "^12.7.5"
  },
  "engines": {

$ yarn generate-clients -g codegen/sdk-codegen/aws-models/acm.json -n
...
copying @aws-sdk/client-acm from /local/home/trivikr/workspace/backup/backup-aws-sdk-js-v3/codegen/sdk-codegen/build/smithyprojections/sdk-codegen/acm.2015-12-08/typescript-codegen to /local/home/trivikr/workspace/backup/backup-aws-sdk-js-v3/clients
Done in 13.50s.

$ sed -n 53,58p clients/client-acm/package.json 
  },
  "devDependencies": {
    "@aws-sdk/service-client-documentation-generator": "3.38.0",
    "@types/node": "^12.7.5"
  },
  "engines": {

Additional context

Discussion in lerna lerna/lerna#1079


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@trivikr
Copy link
Member Author

trivikr commented Jan 5, 2022

Moving to draft till release automation moves to using build:docs
Currently it calls ./node_modules/.bin/typedoc in each client.

@trivikr trivikr marked this pull request as draft January 5, 2022 16:09
@trivikr trivikr changed the title chore(scripts): remove devDeps defined in monorepo root from clients chore(scripts): remove devDeps defined in monorepo root Jan 5, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jan 5, 2022

Codecov Report

Merging #3138 (8343f08) into main (f6a3ef5) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #3138   +/-   ##
=======================================
  Coverage   75.19%   75.19%           
=======================================
  Files         474      474           
  Lines       20721    20721           
  Branches     4755     4755           
=======================================
  Hits        15581    15581           
  Misses       5140     5140           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f6a3ef5...8343f08. Read the comment docs.

@trivikr trivikr marked this pull request as ready for review January 5, 2022 20:10
@@ -35,6 +35,11 @@ const mergeManifest = (fromContent = {}, toContent = {}) => {
const merged = {};
for (const name of Object.keys(fromContent)) {
if (fromContent[name].constructor.name === "Object") {
if (name === "devDependencies") {
// Remove devDeps defined in monorepo root
const devDepsInRoot = ["downlevel-dts", "rimraf", "typedoc", "typescript"];
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you checked if we can also remove the @aws-sdk/service-client-documentation-generator?

Copy link
Member Author

Choose a reason for hiding this comment

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

We should be able to remove @aws-sdk/service-client-documentation-generator if it's defined in root package.json
We can take it up in a separate PR as it's an internal dependency and will need additional testing with docs generation.

@AllanZhengYP AllanZhengYP self-requested a review January 5, 2022 22:10
@trivikr trivikr merged commit f2dab56 into aws:main Jan 5, 2022
@trivikr trivikr deleted the clients-remove-depDeps-defined-in-package-root branch January 5, 2022 23:35
@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 20, 2022
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.

None yet

3 participants