Implement basic Codex styles to DataTables #871
Conversation
alistair3149
changed the title
alistair3149
changed the title
|
@thomas-topway-it could you have a look at this? |
| lengthMenuLabels[showAll] = mw.msg( 'srf-ui-datatables-label-rows-all' ); | ||
| } | ||
| // Format value into readable label | ||
| for (var i = 0; i < lengthMenuLabels.length; i++) { |
There was a problem hiding this comment.
There is some inconsistent code style spacing in the new code
There was a problem hiding this comment.
It was my bad, I was used to the spacing scheme in other MW projects.
Would it be helpful to set up eslint and stylelint for the SRF down the line? There would probably be a lot of violations but it would be great to enforce some consistency in code styling.
There was a problem hiding this comment.
Oh, the code here does not use the normal MW style? I would just use the standard MW style. The code here comes from all kinds of random contributors, so not surprising it deviates at places.
Yes, having some CS rules could help... though I wonder how much they would get in the way here. Having the option to merge something slightly inconsistent is nice, because repeatedly harassing new contributors over spaces isn't likely to endear them to the project. So shrug
There was a problem hiding this comment.
Almost all the JS that I worked on within SRF isn't using standard code styles (e.g. whitespace in line 565 is standard MW styles, the rest of the files without whitespace isn't). It might be safe to assume that majority of the code do not conform to the styles based on the age of the code themselves.
I am a bit torn about this because if we are to enable CS rules, the CI will light up like a Christmas tree... I don't have the bandwidth to do it at the moment but perhaps some day in the future.
Recording.2024-12-08.044136.mp4
I will make a follow-up patch to clean up the icons