Skip to content

Conversation

@f-f
Copy link
Member

@f-f f-f commented Jan 19, 2026

This PR stems from a note I left in here:

As an aside, we should patch memoisedGetPackageDependencies to respect the offline flag, since something that is not cached might hit the network. We can push the failures even lower in the stack, but I don't know off the top of my head if that would cause trouble (e.g. if we gate all the Spago.Registry functions with it). The Spago.Git module has it at least, we just haven't propagated in all the low-level places that hit the network since we usually handle it at high level.

..so this PR does just that. We add offline-ness checking a little deeper in the stack so that we don't accidentally hit the network.

@f-f f-f requested a review from fsoikin January 19, 2026 09:57
Copy link
Collaborator

@fsoikin fsoikin left a comment

Choose a reason for hiding this comment

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

Looks cool. I left some whenM/unlessM suggestions, but not really important, just reducing the boilerplace.

Also: what about testing this new behavior?

@f-f
Copy link
Member Author

f-f commented Jan 22, 2026

@fsoikin testing this behaviour would be cumbersome, as you would need to clear out the global spago cache, where the registry repos are stored. I don't think we do it anywhere in the test suite, and the location is OS-dependent, etc. I have attempted before and I didn't like it - I guess we could try again, but I also don't want to make it too hard to merge improvements just because we don't have the test infrastructure for it.

@f-f f-f merged commit 1c125c9 into master Jan 22, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants