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
Update: add question to confirm downgrade (fixes #8870) #8911
Conversation
LGTM |
@mysticatea, thanks for your PR! By analyzing the history of the files in this pull request, we identified @IanVS, @not-an-aardvark and @gyandeeps to be potential reviewers. |
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.
Nice work on this, @mysticatea! Implementation LGTM once I wrapped my head around what checkLocalESLintVersion
does, so I made a suggestion for a name that might help clarify that.
lib/config/config-initializer.js
Outdated
* @param {Object} answers The answers object. | ||
* @returns {boolean} `true` if the local ESLint is not found or it satisfies the required version of the chosen shareable config. | ||
*/ | ||
function checkLocalESLintVersion(answers) { |
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.
To make the meaning of this method's boolean return value more apparent, what do you think of renaming this to hasESLintVersionConflict
and inverting the return values? (This would allow you to drop the negation when calling this function on line 452.)
localESLintVersion = null; | ||
}); | ||
|
||
it("should returns true.", () => { |
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.
Tiny nit: "should return" (no "s") here and a couple times below
LGTM |
I updated this PR as following suggestions:
|
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, thanks @mysticatea!
What is the purpose of this pull request? (put an "X" next to item)
[X] Bug fix : fixes #8870
What changes did you make? (Give an overview)
This PR adds a question to confirm that people allow
eslint --init
downgrades ESLint.(Click to open) In a `Yes` case:
(Click to open) In a `No` case:
This question is shown only if
peerDependencies
of the chosen config is mismatched with the current local ESLint.peerDependencies
, it's not shown.(Click to open) In a not shown case:
Is there anything you'd like reviewers to focus on?
Nothing in particular.