-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
feat(linter): rename @nx/eslint-plugin-nx to @nx/eslint-plugin #16420
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
a03f5c5
to
431de7c
Compare
431de7c
to
7c43180
Compare
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.
Can you update codeowners for the eslint-plugin-nx directory?
7c43180
to
5251479
Compare
a3ccc35
to
863f642
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
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.
Fixed :)
863f642
to
6af5e2e
Compare
"root": "/packages/eslint-plugin-nx", | ||
"source": "/packages/eslint-plugin-nx/src" | ||
"name": "eslint-plugin", | ||
"packageName": "@nrwl/eslint-plugin", |
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 is invalid - should be @nrwl/eslint-plugin-nx
@@ -32,6 +32,5 @@ | |||
] | |||
} | |||
} | |||
}, | |||
"implicitDependencies": ["eslint-plugin-nx"] |
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.
Why was this removed?
If you remove this, then e2e-linter
project needs implicit dependency of eslint-plugin-nx
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 is the legacy package, it only has a dependency on @nx/linter
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.
Missed that suffix. Tnx
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.
Left few minor comments
6af5e2e
to
dffa7ca
Compare
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.
Looks good now, thanks!
@@ -5,7 +5,7 @@ export default { | |||
}, | |||
moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx', 'html'], | |||
globals: {}, | |||
displayName: 'eslint-plugin-nx', | |||
displayName: 'packages-eslint-plugin', |
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.
non-blocking: Does this need the packages-
prefix? We don't use that elsewhere for this
This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request. |
Current Behavior
The new
@nx/eslint-plugin-nx
package means the rules look like@nx/nx/enforce-module-boundaries
which is redundant.Expected Behavior
The new
@nx/eslint-plugin-nx
package is now@nx/eslint-plugin
means the rules look like@nx/enforce-module-boundaries
which is better.Related Issue(s)
Fixes #