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.

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

document cache #80

Merged
merged 3 commits into from
May 4, 2018
Merged

document cache #80

merged 3 commits into from
May 4, 2018

Conversation

ralphtheninja
Copy link
Member

No description provided.

@ralphtheninja ralphtheninja requested a review from vweevers May 3, 2018 11:22
@mathiask88
Copy link
Member

What about documenting the nolocal option together with these changes?

@ralphtheninja
Copy link
Member Author

What about documenting the nolocal option together with these changes?

I had completely forgotten about that. It seems to have originated from prebuild. @mathiask88 Do you remember the story behind it?

@ralphtheninja
Copy link
Member Author

Aaah here we go. prebuild/prebuild@72a161a

@ralphtheninja
Copy link
Member Author

I think it was incorrectly documented here prebuild/prebuild@ce6358c#diff-04c6e90faac2675aa89e2176d2eec7d8R190 .. since .nolocal == true would still check cached prebuilds, but not locally cached prebuilds.

I'm wondering why it was added in the first place.

@ralphtheninja
Copy link
Member Author

@mathiask88 I'm thinking we could remove nolocal since no one seems to use it anyway. We would have to release a new major again though.

@mathiask88
Copy link
Member

mathiask88 commented May 3, 2018

Yes you are right. Does the local cache check makes any sense for prebuild-install? Maybe we can remove that and move the nolocal?

Edit: Or remove both :D

@ralphtheninja
Copy link
Member Author

@brett19 Mind helping us shed some light into this? 😄

@ralphtheninja
Copy link
Member Author

Yes you are right. Does the local cache check makes any sense for prebuild-install? Maybe we can remove that and move the nolocal?

I'm thinking the same, that it might not make sense anymore.

@ralphtheninja
Copy link
Member Author

@mathiask88 I think I'll update the nolocal stuff in a separate PR.

@ralphtheninja ralphtheninja merged commit d90cfac into master May 4, 2018
@ralphtheninja ralphtheninja deleted the cache branch May 4, 2018 15:15
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.

2 participants