Skip to content

replaces jscs with eslint#1942

Closed
PseudoNinja wants to merge 4 commits into
jquery:mainfrom
PseudoNinja:feature/eslint
Closed

replaces jscs with eslint#1942
PseudoNinja wants to merge 4 commits into
jquery:mainfrom
PseudoNinja:feature/eslint

Conversation

@PseudoNinja
Copy link
Copy Markdown

@PseudoNinja PseudoNinja commented Dec 18, 2020

Component: Core Dependency Library

The JSCS project has been archived and now contains security flaws. Migrated to ESLint to continue to provide linting service while alleviating the security concerns.

Fixes #15393

@melloware
Copy link
Copy Markdown

Nice concise PR!

Base automatically changed from master to main February 19, 2021 19:58
Copy link
Copy Markdown
Member

@mgol mgol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. I think this needs some changes, though, and that may generate source changes that will be non-trivial. It's worth checking, though.

I'd expect this to remove JSHint as well.

Comment thread eslint.json Outdated
Comment thread eslint.json Outdated
@@ -0,0 +1,24 @@
{
"extends": "eslint:recommended",
Copy link
Copy Markdown
Member

@mgol mgol Feb 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UI should use the jQuery config, similarly as jQuery itself does.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Im not sure we can get away from using the standardized eslint configuration file

@PseudoNinja
Copy link
Copy Markdown
Author

Went ahead and removed jshint from the package. Let me know if there are any other changes you would like to see

Copy link
Copy Markdown
Member

@mgol mgol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current preset is a blocker issue to me, unfortunately.

Comment thread Gruntfile.js
src: [ "tests/**/*.js" ]
}
}
target:["demos/**/*.js", "build/**/*.js", "ui/**/*.js"]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't follow the jQuery code style with regards to spacing; doesn't ESLint verify it?

Comment thread eslint.json
@@ -0,0 +1,24 @@
{
"extends": "eslint:recommended",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jQuery projects are not supposed to use this preset but the jQuery one. This is to me a prerequisite to the migration, we can't migrate to a preset that doesn't verify rules jQuery cares about.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which preset would you recommend then?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mgol
Copy link
Copy Markdown
Member

mgol commented Jun 2, 2021

I did the ESLint work in #1958, so I'm closing this PR.

@mgol mgol closed this Jun 2, 2021
@PseudoNinja PseudoNinja deleted the feature/eslint branch June 3, 2021 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants