feat(aws-cdk): auto-detect package manager when creating init templates #23259

ShivamJoker · 2022-12-07T09:15:22Z

Fixes #23205

I have run the build and test command

All Submissions:

Have you followed the guidelines in our Contributing guide?

Adding new Construct Runtime Dependencies:

This PR adds new construct runtime dependencies following the process described here

New Features

Have you added the new feature to an integration test?
- Did you use yarn integ to deploy the infrastructure and generate the snapshot (i.e. yarn integ without --dry-run)?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

gitpod-io · 2022-12-07T09:15:28Z

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

ShivamJoker · 2022-12-07T09:22:03Z

@peterwoodworth I am not sure what changes I should do in the test file.
Can you please help me with that?

peterwoodworth · 2022-12-07T20:39:16Z

I'm personally not that familiar with our CLI testing, but we have a page on it. Check it out and see if this clears anything up for ya

comcalvi

I'm not a fan of adding new dependencies for this. What problems is npm causing? Could you expand on the original issue a bit more to explain the need for this?

mrgrain · 2022-12-09T19:34:35Z

@ShivamJoker I've renamed the title as this as this is clearly a feature. But don't just go off making the linter happy. Let's first decide if we actually want that.

ShivamJoker · 2022-12-09T19:38:01Z

@ShivamJoker I've renamed the title as this as this is clearly a feature. But don't just go off making the linter happy. Let's first decide if we actually want that.

I wasn't sure, there are so many things going on here. But no worries.

ShivamJoker · 2022-12-09T19:43:25Z

I'm not a fan of adding new dependencies for this. What problems is npm causing? Could you expand on the original issue a bit more to explain the need for this?

@comcalvi I didn't add the library I took the functionality and put it in the utils.

Tools like Vite and Astro installs the dependencies using the package manager which was used to create the project, so ideally this should be implemented in CDK init command as well.

What problems is npm causing?

If I use yarn/pnpm I should be prompted to install dependencies with yarn/pnpm not npm (which is the current behaviour).

yarn.lock

TheRealAmazonKendra · 2022-12-09T01:59:47Z

packages/aws-cdk/lib/util/get-package-manager.ts

@@ -0,0 +1,20 @@
+const pmFromUserAgent = (userAgent: string) => {


The functions in this file need to be unit tested and docstring should be added here.

Based off this code, I don't have any idea what userAgent could contain that this function is splitting and manipulating.

What would a doc string look like for a string?

TheRealAmazonKendra · 2022-12-09T02:07:37Z

packages/aws-cdk/lib/util/get-package-manager.ts

+  const separatorPos = pmSpec.lastIndexOf('/');
+  const name = pmSpec.substring(0, separatorPos);
+  return {
+    name: name === 'npminstall' ? 'cnpm' : name,


Why is cnpm assigned when name is npminstall?

Because internally cnpm uses npminstall.

https://github.com/cnpm/cnpm#install-with-original-npm-cli

aws-cdk-automation · 2022-12-11T00:11:50Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation · 2022-12-12T00:09:47Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation · 2022-12-12T13:04:28Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: e5cee1c
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

comcalvi

there should be no yarn.lock changes here, we have automation that performs those updates regularly. I do not think the burden of more dependencies is worthwhile for a feature that does not add functional capabilities for the CLI.

Does the CLI break when using pnpm to install dependencies? yarn seems to work without issues.

comcalvi · 2022-12-15T21:32:00Z

packages/aws-cdk/lib/util/get-package-manager.ts

+  const pmSpec = userAgent.split(' ')[0];
+  const separatorPos = pmSpec.lastIndexOf('/');
+  const name = pmSpec.substring(0, separatorPos);


magic. We need a quick comment showing example(s) of what this string is expecting

comcalvi · 2022-12-15T21:32:37Z

packages/aws-cdk/lib/platform-warnings.ts

-import * as logging from './logging';
 import * as fs from 'fs-extra';
+import * as logging from './logging';


why these import order changes?

aws-cdk-automation · 2022-12-16T00:09:26Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation · 2022-12-17T00:09:32Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation · 2022-12-18T00:09:39Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation · 2022-12-19T00:10:02Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation · 2022-12-21T00:08:36Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation · 2022-12-22T00:08:37Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation · 2022-12-23T00:09:41Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation · 2022-12-24T00:08:23Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation · 2022-12-25T00:09:20Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation · 2022-12-26T00:09:12Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation · 2022-12-27T00:09:11Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation · 2022-12-28T00:09:08Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation · 2022-12-29T00:08:25Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation · 2022-12-30T00:08:24Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation · 2022-12-31T00:07:55Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation · 2023-01-01T00:10:12Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation · 2023-01-02T00:08:37Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation · 2023-01-03T00:08:14Z

This PR has been in the MERGE CONFLICTS state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

ShivamJoker · 2023-01-03T07:58:24Z

Can someone please mute this bot?

aws-cdk-automation · 2023-01-10T00:09:04Z

This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error.

aws-cdk-automation · 2023-01-10T00:11:06Z

The pull request linter fails with the following errors:

❌ Features must contain a change to a README file.
❌ Features must contain a change to a test file.
❌ Features must contain a change to an integration test file and the resulting snapshot.
❌ CLI code has changed. A maintainer must run the code through the testing pipeline (git fetch origin pull/23259/head && git push -f origin FETCH_HEAD:test-main-pipeline), then add the 'pr-linter/cli-integ-tested' label when the pipeline succeeds.

PRs must pass status checks before we can provide a meaningful review.

refactor(aws-cdk): sort imports

39ec318

github-actions bot added the beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK label Dec 7, 2022

aws-cdk-automation requested a review from a team December 7, 2022 09:15

github-actions bot added effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2 labels Dec 7, 2022

aws-cdk-automation requested changes Dec 7, 2022

View reviewed changes

fix(aws-cdk): use the appropriate package manager instead of npm

6992068

ShivamJoker force-pushed the main branch from b8604c0 to 6992068 Compare December 7, 2022 09:19

ShivamJoker changed the title ~~feat(aws-cdk): use the appropriate package manager instead of npm~~ fix(aws-cdk): use the appropriate package manager instead of npm Dec 7, 2022

comcalvi reviewed Dec 9, 2022

View reviewed changes

mrgrain changed the title ~~fix(aws-cdk): use the appropriate package manager instead of npm~~ feat(aws-cdk): auto-detect package manager when creating init templates Dec 9, 2022

TheRealAmazonKendra requested changes Dec 9, 2022

View reviewed changes

remove types from yarn

e5cee1c

comcalvi requested changes Dec 15, 2022

View reviewed changes

aws-cdk-automation added the closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. label Jan 10, 2023

aws-cdk-automation closed this Jan 10, 2023

		@@ -0,0 +1,20 @@
		const pmFromUserAgent = (userAgent: string) => {

feat(aws-cdk): auto-detect package manager when creating init templates #23259

feat(aws-cdk): auto-detect package manager when creating init templates #23259

Conversation

ShivamJoker commented Dec 7, 2022

All Submissions:

Adding new Construct Runtime Dependencies:

New Features

gitpod-io bot commented Dec 7, 2022

aws-cdk-automation left a comment

Choose a reason for hiding this comment

ShivamJoker commented Dec 7, 2022

peterwoodworth commented Dec 7, 2022

comcalvi left a comment

Choose a reason for hiding this comment

mrgrain commented Dec 9, 2022

ShivamJoker commented Dec 9, 2022

ShivamJoker commented Dec 9, 2022

TheRealAmazonKendra Dec 9, 2022

Choose a reason for hiding this comment

ShivamJoker Dec 11, 2022

Choose a reason for hiding this comment

TheRealAmazonKendra Dec 9, 2022

Choose a reason for hiding this comment

ShivamJoker Dec 11, 2022

Choose a reason for hiding this comment

aws-cdk-automation commented Dec 11, 2022

aws-cdk-automation commented Dec 12, 2022

aws-cdk-automation commented Dec 12, 2022

AWS CodeBuild CI Report

comcalvi left a comment • edited Loading

Choose a reason for hiding this comment

comcalvi Dec 15, 2022

Choose a reason for hiding this comment

comcalvi Dec 15, 2022

Choose a reason for hiding this comment

aws-cdk-automation commented Dec 16, 2022

aws-cdk-automation commented Dec 17, 2022

aws-cdk-automation commented Dec 18, 2022

aws-cdk-automation commented Dec 19, 2022

aws-cdk-automation commented Dec 21, 2022

aws-cdk-automation commented Dec 22, 2022

aws-cdk-automation commented Dec 23, 2022

aws-cdk-automation commented Dec 24, 2022

aws-cdk-automation commented Dec 25, 2022

aws-cdk-automation commented Dec 26, 2022

aws-cdk-automation commented Dec 27, 2022

aws-cdk-automation commented Dec 28, 2022

aws-cdk-automation commented Dec 29, 2022

aws-cdk-automation commented Dec 30, 2022

aws-cdk-automation commented Dec 31, 2022

aws-cdk-automation commented Jan 1, 2023

aws-cdk-automation commented Jan 2, 2023

aws-cdk-automation commented Jan 3, 2023

ShivamJoker commented Jan 3, 2023

aws-cdk-automation commented Jan 10, 2023

aws-cdk-automation commented Jan 10, 2023

comcalvi left a comment •

edited

Loading