Remove eslint custom rules and introduce prettier #5877
Conversation
Hello williamlardier,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Incorrect fix versionThe
Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:
Please check the |
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found. Additional details and impacted files
@@ Coverage Diff @@
## development/9.0 #5877 +/- ##
===================================================
+ Coverage 75.88% 75.90% +0.02%
===================================================
Files 188 188
Lines 11983 11957 -26
===================================================
- Hits 9093 9076 -17
+ Misses 2890 2881 -9
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Incorrect fix versionThe
Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:
Please check the |
eslint.config.mjs
Outdated
There was a problem hiding this comment.
Ideally we should use .editorconfig for the IDE to format correctly.
We should use prettier for formatting stuff like indent.
And use eslint prettier plugin / config to turn off eslint stylistic rules like indent or quotes (they'll be moved and and not enabled by default in v10)
There was a problem hiding this comment.
We can add a prettier yes. But we still want to enforce indentation before v10. How about introducing a prettier with an action to ensure it ran, then when upgrading to v10, we can remove the eslint rules? (actually, nothing to do, if they are removed).
I'll check for the .editorconfig at the same time.
There was a problem hiding this comment.
indentation is the job of prettier as it rewrite the file
There was a problem hiding this comment.
I introduced prettier, now binded to eslint for consistent rules, with an .editorconfig and CI rules to verify the format.
As discussed, because this is a lot of changes, let's wait for unification to complete first.
Overall we really had a lot of formatting inconsistencies, so this will a bit slow down git blames as most of the lines are impacted, but it will be easier to work with integration branches from now on.
There was a problem hiding this comment.
editorconfig needs an IDE extension for some IDE: https://marketplace.visualstudio.com/items?itemName=EditorConfig.EditorConfig
we could include a .vscode/extensions.json (like https://github.com/microsoft/TypeScript/blob/main/.vscode/extensions.json) so all devs could see a popup to install the extension in their vscode.
Same for prettier and eslint could have extensions to format on save.
williamlardier
force-pushed
the
improvement/CLDSRV-723-remove-eslint-custom-rules
branch
from
5d2b194 to
1fe7d14
Compare
williamlardier
changed the title
We should follow the eslint default, completed by the imported scality rules. Any additional rule should add some constraints, like disabling .only tests to make the CI fail. The reason is that we end up being too leniant on the style, and that translates into useless indent (or other style) changes in branches, due to local tooling auto-formatting the code, which make the reviews harder. Issue: CLDSRV-723
- Add Prettier and eslint-config-prettier dependencies - Configure Prettier with single quotes and 120-char line width - Add format and check-format npm scripts - Create .prettierignore to exclude build outputs and binaries - Add .editorconfig for consistent IDE settings across team Issue: CLDSRV-723
- Extend eslint-config-prettier to disable formatting rules - Add check-format step to GitHub Actions workflow - Ensure CI enforces code formatting standards Issue: CLDSRV-723
Update yarn.lock to include new dependencies: - prettier@^3.3.3 - eslint-config-prettier@^9.1.0 Issue: CLDSRV-723
williamlardier
force-pushed
the
improvement/CLDSRV-723-remove-eslint-custom-rules
branch
from
1fe7d14 to
da39f13
Compare
Apply consistent code formatting using Prettier to appropriate file types: - Convert double quotes to single quotes in JS/MJS/JSON files - Standardize indentation to 4 spaces across JS files - Normalize spacing and line breaks with 120-char width - Apply consistent trailing commas and semicolons - Update package.json scripts to exclude YAML files from formatting - Format ~500 JS/JSON/MD files while preserving YAML formatting Issue: CLDSRV-723
williamlardier
force-pushed
the
improvement/CLDSRV-723-remove-eslint-custom-rules
branch
from
da39f13 to
4123c87
Compare
| @@ -46,8 +46,7 @@ const constants = { | |||
| // only public resources | |||
| publicId: 'http://acs.amazonaws.com/groups/global/AllUsers', | |||
| // All Authenticated Users is an ACL group. | |||
| allAuthedUsersId: 'http://acs.amazonaws.com/groups/' + | |||
| 'global/AuthenticatedUsers', | |||
| allAuthedUsersId: 'http://acs.amazonaws.com/groups/' + 'global/AuthenticatedUsers', | |||
There was a problem hiding this comment.
This is because it was before limited to 80 chars, now it's 120, so those strings can be regrouped, we might have a lot like that in the code
| "cover": "nyc --clean --silent yarn run", | ||
| "postcover": "nyc report --report-dir ./coverage/test --reporter=lcov" | ||
| } | ||
| "name": "@zenko/cloudserver", |
There was a problem hiding this comment.
this is 4 indents, not following prettierc
We should follow the eslint default, completed by the imported scality rules.
Any additional rule should add some constraints, like disabling .only tests to make the CI fail.
The reason is that we end up being too leniant on the style, and that translates into useless indent (or other style) changes in branches, due to local tooling auto-formatting the code, which make the reviews harder.
The main problem reported is the indentation: it is varying from file to file, making the code harder to read.
Almost all rules were already followed, so enabling them was not impacting the code at all.
Then we introduce prettier to ensure code style is consistent.
Issue: CLDSRV-723