remove CacheConf; add tests for utils/conf #204
Merged
Conversation
|
Let me updated |
| expect(conf.get('foo')).toBe('bar'); | ||
| expect(conf.store).toEqual({ foo: 'bar' }); | ||
| expect(conf.size).toBe(1); | ||
| }); |
| // resolve and update the state | ||
| Promise.resolve(content).then(html => { | ||
| cacheConf.set(cacheKey, html); |
adnasa
Apr 15, 2018
Member
aah cool.
Isn't it a matter of replacing this with config.set(), since conf actually inherits from Conf ? 
maybe that doesn't work and I'm missing something.
vutran
Apr 15, 2018
Author
Member
I thought about it but conf in itself never expires whereas in this context, we want the item details content to expire since plugin may be querying an API (github README, etc.)
vutran
Apr 15, 2018
Author
Member
I just removed it for now to keep the function cleaner. I think there's a better way to go about caching without having it too coupled with our fetch functions.
|
removing code when it helps less is better. |
|
don't forget the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
This diff removes
utils/CacheConf(temporarily; a todo has been added for reimplementing caching which can be discussed in a new thread).This also includes a spec file for
utils/conf.