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.

Sign up for GitHub

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

Jump to bottom

feat: pnpm import support yarn2 transform #3997

Merged
merged 5 commits into from Nov 24, 2021
Merged

feat: pnpm import support yarn2 transform #3997

merged 5 commits into from Nov 24, 2021

Conversation

fireairforce
Copy link
Member

closes: #3993

@fireairforce fireairforce force-pushed the support-v2 branch 2 times, most recently from 7435f91 to 102ceb8 Compare November 16, 2021 17:03
@fireairforce
Copy link
Member Author

fireairforce commented Nov 16, 2021 •
edited

I need to wait for the pr merged: snyk/nodejs-lockfile-parser#131...

I will execute by myself.

@ylemkimon
Copy link
Member

Note Yarn 2+ lockfile is a valid YAML and can be parsed with any YAML parser.

@fireairforce
Copy link
Member Author

Of course,So maybe we can implement by ourselves

@fireairforce fireairforce force-pushed the support-v2 branch 3 times, most recently from 606f79d to 9179d0a Compare November 23, 2021 07:45
@fireairforce fireairforce changed the title (WIP)feat: pnpm import support yarn2 transform feat: pnpm import support yarn2 transform Nov 23, 2021
@fireairforce
Copy link
Member Author

image

@zkochan CI failed for the network error?

zkochan
zkochan reviewed Nov 23, 2021
View reviewed changes
pnpm-lock.yaml Outdated Show resolved Hide resolved
zkochan
zkochan reviewed Nov 23, 2021
View reviewed changes
packages/plugin-commands-installation/src/import.ts Outdated
if (lockJsonFile.type === 'success') {
return lockJsonFile.object
} else {
throw new PnpmError('GET_YARN_LOCKFILE_ERR', `Failed With ${lockJsonFile.type}`)
Copy link
Member

Choose a reason for hiding this comment

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

Don't put _ERR in the error code because each error code is automatically prepended with ERR_PNPM_

packages/plugin-commands-installation/src/yarnUtil.ts Outdated
@@ -0,0 +1,90 @@
/**
Copy link
Member

Choose a reason for hiding this comment

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

maybe let's make a subdirectory for the import command and put this file there.

and add yourself as codeowner of it to .github/CODEOWNERS

/packages/plugin-commands-installation/src/import/ @fireairforce

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, thx, i will do this

@fireairforce
Copy link
Member Author

image

the ci failed but it works fine at my local env..

@zkochan
Copy link
Member

zkochan commented Nov 23, 2021

I'll check later.

@fireairforce
Copy link
Member Author

I have updated the case, it will run successfully.

@zkochan zkochan merged commit 3b4dc17 into pnpm:main Nov 24, 2021
@zkochan zkochan added this to the v6.23 milestone Nov 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zkochan zkochan zkochan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v6.23
Development

Successfully merging this pull request may close these issues.

pnpm import fails with yarn v2
3 participants
@fireairforce @ylemkimon @zkochan