这是indexloc提供的服务,不要输入任何密码
Skip to content

refactor: Prune prefactor #1689

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

Merged
merged 3 commits into from
Aug 12, 2022
Merged

Conversation

chris-olszewski
Copy link
Member

Before I add support for pnpm in the prune command I wanted to add some more tests and take a quick pass at deduping some code.

  • Add e2e test for --docker flag
  • Move the check for if a package manager supports the prune command into the PM abstraction
  • Dedupe the --docker vs vanilla codepaths

Unsure of the signature of CanPrune as it could be simplified to just return an error if the package manager doesn't support, but that feels off to me. Would appreciate any thoughts on this.

@vercel
Copy link

vercel bot commented Aug 12, 2022

@chris-olszewski is attempting to deploy a commit to the Vercel Team on Vercel.

A member of the Team first needs to authorize it.

@chris-olszewski chris-olszewski marked this pull request as ready for review August 12, 2022 16:27
@gsoltis gsoltis added the pr: fixship A PR which is approved with notes and does not require re-review. label Aug 12, 2022
Copy link
Contributor

@gsoltis gsoltis left a comment

Choose a reason for hiding this comment

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

LGTM, just one minor fix-ship


// CanPrune returns if turbo can produce a pruned workspace. Can error if fs issues occur
func (pm PackageManager) CanPrune(projectDirectory fs.AbsolutePath) (bool, error) {
if canPrune := pm.canPrune; canPrune != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you need canPrune here. Is this equivalent to if pm.canPrune != nil { ?

@chris-olszewski chris-olszewski merged commit 97320af into vercel:main Aug 12, 2022
@weyert
Copy link
Contributor

weyert commented Aug 12, 2022

Are you planning to use pnpm deploy?

@chris-olszewski
Copy link
Member Author

Are you planning to use pnpm deploy?

Probably not, from what I can tell pnpm deploy will only take care of copying the package and it's internal deps which Turbo already does for Yarn. The larger part of implementing turbo prune for pnpm is generating a partial lockfile.

@nathanhammond
Copy link
Contributor

(Erroneous mention.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: fixship A PR which is approved with notes and does not require re-review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants