-
Notifications
You must be signed in to change notification settings - Fork 2
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
Improvement/wrlgs 12 dep upgrades #169
Conversation
Hello williamlardier,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
|
994e264
to
0ab4ca3
Compare
.eslintrc
Outdated
{ "extends": "scality" } | ||
{ | ||
"extends": "scality", | ||
"parserOptions": { |
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't we move these the scality/guidelines package, to avoid copying/duplicating and maintaining different styles?
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.
Actually I think we can just remove them and adapt the code. I'll give it a quick try.
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.
Done, like that we don't need to change scality guidelines by disabling good practice rules...
package.json
Outdated
"markdownlint-cli": "^0.27.1", | ||
"mocha": "^8.4.0", | ||
"@babel/eslint-parser": "^7.25.1", | ||
"eslint": "^8.57.0", |
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.
eslint 9 ?
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.
We cannot, it requires node 18+, but then we cannot use it in our projects because they're not yet upgraded to node 20, which would make this dependency not usable... Do we want to upgrade to eslintv9 now?
error @eslint/config-array@0.18.0: The engine "node" is incompatible with this module. Expected version "^18.18.0 || ^20.9.0 || >=21.1.0". Got "16.20.2"
error Found incompatible module.
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.
Now it uses eslint v9, I also removed almost all devdeps, are they are unused, we should only rely on the scality eslint configuration... This will simplify things and then we can really enforce a coding style everywhere.
0ab4ca3
to
fb7d4b1
Compare
fb7d4b1
to
e67c765
Compare
e67c765
to
a1f0f91
Compare
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
|
a1f0f91
to
9df0245
Compare
9df0245
to
2d43c65
Compare
2d43c65
to
e9f5fa1
Compare
Issue: WRLOGS-12
e9f5fa1
to
7bf334c
Compare
/approve |
I have successfully merged the changeset of this pull request
The following branches have NOT changed:
Please check the status of the associated issue WRLGS-12. Goodbye williamlardier. The following options are set: approve |
Issue: WRLGS-12