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
New: Implement cacheStrategy (refs eslint/rfcs#63) #14119
New: Implement cacheStrategy (refs eslint/rfcs#63) #14119
Conversation
Hi @chambo-e!, thanks for the Pull Request The pull request title isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.
Read more about contributing to ESLint here |
1 similar comment
Hi @chambo-e!, thanks for the Pull Request The pull request title isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.
Read more about contributing to ESLint here |
61a1967
to
15aad06
Compare
Hi @chambo-e!, thanks for the Pull Request The pull request title isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.
Read more about contributing to ESLint here |
Hi @chambo-e!, thanks for the Pull Request The pull request title isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.
Read more about contributing to ESLint here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good, just a couple of things to clarify.
…ring instead of configHelper object
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I'd like another set of eyes on this before merging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to also fix the behavior in the case that is described here:
Edit: Not in this PR.
@mdjermanovic you indicated in the RFC that this wasn't a blocker. Is that something we can manage via a separate issue rather than blocking this PR for it? |
Agreed we shouldn't address that in this PR. The issue might get fixed in I meant it shouldn't be a blocker for this feature. If it isn't fixable, we can tell users to always delete the cache file before changing the strategy. |
@mdjermanovic @nzakas published |
@royriojas thanks! @chambo-e can you update |
…ject Updated file-entry-cache to ^6.0.1 Also add tests outside of the cli-engine
Hello 👋
Prerequisites checklist
What is the purpose of this pull request? (put an "X" next to an item)
[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[X] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:
What changes did you make? (Give an overview)
This is a follow up of #13713 which seems abandoned.
I corrected the tests by binding every stubs to the newly created sinon sandbox
Is there anything you'd like reviewers to focus on?