fix: show type on invalid semver error #559
fix: don't specify eslint plugin in config because already there from…
… base config
- Loading branch information
commit 7acab3450593150d847633341bd05ef48c90880f
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,6 @@ | ||
| 'use strict' | ||
|
|
||
| module.exports = { | ||
| rules: { | ||
| 'import/no-extraneous-dependencies': [ | ||
| 'error', | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
this may not be required. checking
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's already in the base ruleset https://github.com/npm/eslint-config/blob/9e87b58d9e049004c802239cab1fabbca67caf74/lib/index.js#L213
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.
Ah yep. I think this config is useful though? The default would allow importing from
devDependenciesThere 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.
The file is already imported as a plugin, so the extra dependency isn't needed. I think a single new
ruleis all we need here with anallowof./testandmap.js.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.
cf https://github.com/npm/cli/blob/3d5bbcc86c484ef0b6c04ee0236c1072d36915d6/.eslintrc.local.json#L3 or https://github.com/npm/cacache/blob/de12400af2ab452f1e75a15edaf7ea51b294667b/.eslintrc.local.js#L4
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.
Then this needs to be addressed in template-oss, that's not a rule that we'd want to only apply to a single one of our over 80 repos.
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.
Opened npm/template-oss#316
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.
@wraithgar so given this doesn't follow the standard structure npm/eslint-config#67 doesn't have any effect here, so is the custom config in this PR good to go?
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.
Yeah we'll need a custom config but this one looks to be a bit different in approach than the one we landed on in eslint-config. We should pattern it after the one there but instead of
libandbinit should bebin,classes,functions,internal, andranges(and now we begin to see why we standardized onlib).The way this PR currently does it is by completely bypassing the
no-extraneous-dependenciesrule for the test files, which is a regression.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.
Yep. done!