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

rfc 5861 (stale-if-error, stale-while-revalidate) #30

Merged
merged 6 commits into from
Mar 1, 2020

Conversation

sithmel
Copy link
Contributor

@sithmel sithmel commented Feb 15, 2020

solves #27

@sithmel
Copy link
Contributor Author

sithmel commented Feb 15, 2020

stale-if error works fine in cacheable-request https://github.com/lukechilds/cacheable-request/issues/94. Without changing the code. I have created a test case locally.

index.js Outdated Show resolved Hide resolved
index.js Show resolved Hide resolved
@kornelski
Copy link
Owner

kornelski commented Feb 26, 2020

Sorry, GitHub UI is stupid and I didn't notice that my comments weren't public yet

@sithmel
Copy link
Contributor Author

sithmel commented Feb 27, 2020

I also added an easy way to mock time. Tests were randomly breaking otherwise.

@kornelski
Copy link
Owner

Please don't add unrelated functionality to a PR.

I don't like the mocking change. The now method exist specifically to be monkey-patched in tests, so it doesn't need another way of being patched.

@sithmel
Copy link
Contributor Author

sithmel commented Mar 1, 2020

I have reverted the mock. I missed to realise that there was already a way. Thanks for spotting that!

I had to monkey patch now for a single test that was failing in CI.

@kornelski kornelski changed the title minor refactoring, rfc 5861 (stale-if-error, stale-while-revalidate) rfc 5861 (stale-if-error, stale-while-revalidate) Mar 1, 2020
@kornelski kornelski merged commit 1b35980 into kornelski:master Mar 1, 2020
@kornelski
Copy link
Owner

Thank you

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