Skip to content

chore: use neostandard as base for linting #5302

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

chore: use neostandard as base for linting #5302

wants to merge 2 commits into from

Conversation

voxpelli
Copy link
Member

PR Checklist

Overview

Since it was requested that I took time to do this if I wanted this done, then here's an initial migration to neostandard

@voxpelli voxpelli force-pushed the chore/use-neostandard branch from 67a4bda to 0ba51f4 Compare March 17, 2025 15:37
@voxpelli voxpelli requested review from a team March 17, 2025 15:40
@voxpelli voxpelli mentioned this pull request Mar 17, 2025
Open
@voxpelli
Copy link
Member Author

Could possibly add noStyle: true to try to get the diff smaller

mark-wiemer
mark-wiemer reviewed Mar 17, 2025
View reviewed changes
Copy link
Member

@mark-wiemer mark-wiemer left a comment

Choose a reason for hiding this comment

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

Aside from the comments, can confirm all this does is change var to const or let and make some whitespace changes

lib/cli/options.js
@@ -239,14 +239,14 @@ const loadPkgRc = (args = {}) => {
configData = fs.readFileSync(filepath, 'utf8');
} catch (err) {
// If `args.package` was explicitly specified, throw an error
if (filepath == args.package) {
if (filepath === args.package) {
Copy link
Member

Choose a reason for hiding this comment

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

are double-equals and triple-equals always equivalent in this case?

Copy link
Member Author

Choose a reason for hiding this comment

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

Since filepath is a string that should be the case

package.json
@@ -128,8 +127,7 @@
"chai": "^4.3.4",
"coffeescript": "^2.6.1",
"cross-env": "^7.0.2",
"eslint": "^8.56.0",
"eslint-plugin-n": "^17.15.1",
"eslint": "^9.22.0",
Copy link
Member

Choose a reason for hiding this comment

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

Just commenting here as a reminder that this PR updates to ESLint v9 (might otherwise get lost in all the other changes)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I was surprised that we hadn't already updated

@voxpelli
Copy link
Member Author

There are still many warnings that should be dealt with, and the comments in the issue suggests that we add noStyle: true

Can also note that this PR improves / fixes how the eslint-plugin-n interacts with ESM

@voxpelli voxpelli mentioned this pull request Mar 18, 2025
Closed
@voxpelli
Copy link
Member Author

Style less alternative: #5313

@voxpelli
Copy link
Member Author

Can't spend more time on this, create a PR of your own if there's any interest in neostandard.

@voxpelli voxpelli closed this Mar 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mark-wiemer mark-wiemer mark-wiemer left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Repo: Move to neostandard instead of ESLint recommended
2 participants
@voxpelli @mark-wiemer