-
-
Notifications
You must be signed in to change notification settings - Fork 156
fix: prune after installing #187
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
The post action will execute too late to actually improve any caching. By pruning directly after running pnpm install(s), we make sure the store is as clean as possible before continuing. This means any attempt to cache the pnpm store will be caching a pruned store
The original message made it sound like it checked the store and decided there was nothing to prune, in reality pruning just did not run if run_install was not set.
… and store_prune options
|
Adding built-in caching makes pruning in the post action a great idea! So #188 is to prefer over this PR |
I think both can be combined just fine. |
|
So by default we will always run prune after install? But this is a waste unless node_modules is actually cached. |
True. Would it make sense to merge #188 first, and I can revise this PR to prune by default if you use the new caching, or add option if you use some other caching? |
|
Was looking at refactor this PR to do pruning right after install still, but still keep the postaction around for the caching. But with built in caching, pruning right before the caching is what makes sense. If we don't want to add pruning right after install by default to potentially aid people who use their own caching, then I'd say there's probably no benefit to any version of this PR. |
|
@oBusk You closed this PR already? |
|
Since your last comment did not imply that you intentionally closed this PR. I am going to assume that you pressed the wrong button in haste. Re-opening. |
@KSXGitHub yeah, like I said, with the caching, it makes sense to prune right before the caching. Helpful part of this PR could be to prune by default to help people who use their own caching, but if we don't want to do pruning by default, see no value 😄 |
|
I see. I guess custom cache steps require custom prune steps. It's not terribly hard. So I'm closing this again. |
Closes #185
Removes the "post" action, since it is hard to benefit from it for caching purposes.
Instead shifts to run
pnpm store prunedirectly after executingrun_install(s), if any.Also added an option to the action,
store_prunewhich can be set tofalseto disable this pruning.Considerations
Something I'm unsure of, is if some use of the
run_installarray/object could be used to install something that would be prunable, meaning that the automatic pruning would then remove what you just installed? But installing globally should not make it prunable, I believe? If there's some expected action of running pruning directly followingrun_install, would it be enough to add to the documentation to explain that you might want tostore_prune: falseif you do anything weird like that? Or it'd be possible to run prune automatically ifrun_installis true, and not if it's some kind of advanced setup (array/objects)