Skip to content

Conversation

@atrovato
Copy link

@atrovato atrovato commented Feb 6, 2021

Based on #155 comment: split PR eslint first :)

.eslintrc.js Outdated
}
],
"semi": "error"
semi: "error"
Copy link
Author

Choose a reason for hiding this comment

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

@rzr you commented:

please explain why this was changed ?

As file content is json, quotes are not required here.

@atrovato atrovato force-pushed the improve-eslint branch 3 times, most recently from b4911cf to 7093683 Compare February 6, 2021 09:30
"scripts": {
"lint": "eslint '**/*.js'",
"lint": "eslint \"**/*.js\"",
"lint-fix": "eslint \"**/*.js\" --fix",
Copy link
Author

Choose a reason for hiding this comment

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

I use \" to be able to run on windows env.

@rzr
Copy link

rzr commented Feb 9, 2021

Please add a change to ignore modules too ie :

https://github.com/abandonware/noble/pull/171/checks?check_run_id=1844386700#

s/home/runner/work/noble/noble/node_modules/mocha/lib/mocha.js:1215for await (const fixtureFn of fixtureFns) {^^^^^

@atrovato atrovato closed this Feb 11, 2021
@atrovato atrovato reopened this Feb 11, 2021
@atrovato
Copy link
Author

This error is not linked to eslint but to node 8 build. Should I do in this PR or in a separate one?

@atrovato
Copy link
Author

With eslint 7, node 8 is no longer supported

https://eslint.org/docs/user-guide/migrating-to-7.0.0#nodejs-8-is-no-longer-supported

@atrovato
Copy link
Author

And As of v8.0.0, Mocha requires Node.js v10.12.0 or newer.

https://mochajs.org/#installation

@atrovato
Copy link
Author

Ok I changed nodepackage.yml to ignore lin on node 8, and to use mocha 7.2.0 on node 8.

@rzr
Copy link

rzr commented Feb 13, 2021

sounds good to me

Copy link

@rzr rzr left a comment

Choose a reason for hiding this comment

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

please open new pr with this change : b76fa34#diff-b29da93bc686f97c01ba07c233850a8c2a21d53cbe9bfc153f905640106da66b and then your eslint fixes then you can rebase everything to make sure nothing break

@atrovato atrovato mentioned this pull request Feb 13, 2021
@atrovato atrovato force-pushed the improve-eslint branch 3 times, most recently from 6c0bcdc to 1dde9ab Compare February 14, 2021 14:13
Copy link

@rzr rzr left a comment

Choose a reason for hiding this comment

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

please rebase with only eslint changes

@atrovato
Copy link
Author

Done :)

@rzr rzr merged commit 82f2b0e into abandonware:master Feb 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants