Closed (fixed)
Project:
Module Filter
Version:
4.x-dev
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
1 Feb 2023 at 15:18 UTC
Updated:
15 Dec 2023 at 16:29 UTC
Jump to comment: Most recent
Address standards in these js files
js/module_filter.modules_uninstall.js
js/module_filter.permissions.js
js/module_filter.update_status.js
NA
Address issues or document why we can't
Implement
Test each file
Review
NA
NA
NA
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
jonathan1055 commentedJust made some changes to get things started. Problem is I can't run eslint locally. So there will be plenty of things still wrong with this, plus I've only tackled the easy bits first.
Comment #5
jonathan1055 commentedFirst run took the coding standards for these three files down to 49 (27 fewer than branch)
Comment #6
jonathan1055 commentedSecond run - 31 messages (41 fewer than branch)
Comment #7
jonathan1055 commentedDown to 27 messages (49 fewer than branch)
Comment #8
jonathan1055 commentedTwo files clean. Just three messages remaining in module_filter.permissions.js
Comment #9
smustgrave commentedCan convert them or just to silence the errors. Dealers choice.
Comment #10
jonathan1055 commentedI'm going to pick this up again and see what the current state is. From the branch pipeline eslint we have
In the MR26 we have the same set of five files but the log says
149 problems (149 errors, 0 warnings)Most of these are prettier formatting warnings, which will be good to fix, but we need to separate them out on a separate issue. So I'm going to set them to be ignored in this MR, to see what is left over.
Comment #11
jonathan1055 commentedThat removed all but 1 of the warnings. We now get just
So this is a new warning, because jquery.winnow.js was clean back in May when you fixed #3338338: Javascript coding standards (winnow.js and module_filter.js). But we can fix that here.
We also do not get the three remaining warning I listed in #8 above. Did your recent work alter/remove some the rules?
Comment #12
jonathan1055 commentedThat fixed it. The pipeline with *.js in .prettierignore runs green for eslint. So this file can be put back (so .js are not ignored) and then this MR can be merged.
I will open a separate issue to look at the prettier warnings. It would be nice to have the branch test passing green.
Comment #13
jonathan1055 commentedThis is RTBC
The follow-up is #3405485: Fix prettier formating warnings in javascript files
Comment #15
smustgrave commentedThanks for keeping this one moving!
Comment #16
jonathan1055 commentedand thanks for merging.
#3405485: Fix prettier formating warnings in javascript files will now be easier to work on, too.