chore: add the oxfmt formatter #1392
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1392 +/- ##
==========================================
+ Coverage 93.70% 93.73% +0.02%
==========================================
Files 111 111
Lines 4387 4626 +239
Branches 1389 1557 +168
==========================================
+ Hits 4111 4336 +225
- Misses 276 290 +14 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Size Change: +675 B (+0.5%) Total Size: 135 kB
|
|
The currently known pair is eslint and prettier, I'm not sure keen to introduce this tool which seems to be a bit "too" new. |
|
Eslint does not do formatting of spaces. |
1 similar comment
|
Eslint does not do formatting of spaces. |
|
Doesn't |
|
Moreover, I think stylistic took that part of eslint and there are some "default" recommendations: I'm not saying it the best solution, I'm just offering alternatives. |
|
I am not talking about indents, I am talking about things like below comments. Eslint is a linter and the job of a linter is a bit different from a formatter. |
CommanderStorm
left a comment
CommanderStorm
left a comment
There was a problem hiding this comment.
Here are some of the reasons why I think adding a formatter is beneficial
| // there is no support yet but there is a tracking issue | ||
| const maplibreIssue = /https:\/\/github.com\/maplibre\/[^/]+\/issues\/(\d+)/; | ||
| const match = support.match(maplibreIssue); | ||
| const match = support.match(maplibreIssue); |
There was a problem hiding this comment.
Before: One space too many
| return key | ||
| .split('-') | ||
| .map((k) => k.replace(/(.)(.*)/, (_, _1, _2) => `${_1.toUpperCase()}${_2}`)) | ||
| .concat('LayerSpecification') | ||
| .join(''); |
There was a problem hiding this comment.
Before: harder to read due to line length
| writeFileSync( | ||
| 'src/types.g.ts', |
There was a problem hiding this comment.
Before: weird indentation
There was a problem hiding this comment.
Before: seemingly ignored by foematting
| const config: RollupOptions[] = [ | ||
| { |
There was a problem hiding this comment.
Before: inconsistent indentation of brackets
|
I think this requires looking into the eslint configuration deeper, I did a shallow pass. |
|
Fair. I just removed all of https://eslint.org/blog/2023/10/deprecating-formatting-rules/ Has the advantage that the migration to v11 will be simpler as now only 2 deprecations relating to commonjs are remaining. |
Currently, our code is unformatted.
I think adding a formatter would be nice.
Adding it to CI should be a different PR for better reviewability