-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Conversation
@chris-olszewski is attempting to deploy a commit to the Vercel Team on Vercel. A member of the Team first needs to authorize it. |
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, 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 { |
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 don't think you need canPrune
here. Is this equivalent to if pm.canPrune != nil {
?
3643c2a
to
1a0712e
Compare
Are you planning to use |
Probably not, from what I can tell |
(Erroneous mention.) |
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.
--docker
flag--docker
vs vanilla codepathsUnsure 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.