Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Part of #1266442: [meta] Implement automated code style checks for core.
Once core is cleaned up, qa.drupal.org should take code style into account when determining pass or fail for css coding standards
See #1190252: [573] Use csslint as a weapon to beat the crappy CSS out of Drupal core for current progress, once that is done it can be added to the testbot
Comment | File | Size | Author |
---|---|---|---|
#4 | csslint.xml_.txt | 196.58 KB | attiks |
#4 | i2575497-4.patch | 4.28 KB | attiks |
#3 | i2575497-3.patch | 4.27 KB | attiks |
#2 | i2575497-2.patch | 4.25 KB | attiks |
Comments
Comment #2
attiks CreditAttribution: attiks at Attiks commentedChecks css files using the csslintrc file already present in core.
Comment #3
attiks CreditAttribution: attiks at Attiks commentedNew patch, creating the artifacts dir
Comment #4
attiks CreditAttribution: attiks at Attiks commentedNew patch excluding core/tests since it has css files containing UTF16 chars
Comment #5
elachlan CreditAttribution: elachlan commentedWould it make sense to add csslint to the container definition? Similar to the changes for #2600626: [PP-1] Ensure availability of node/npm in the testrunners?
Comment #6
elachlan CreditAttribution: elachlan commentedCreated a new issue to add csslint to the container #2605308: Add csslint to the testrunners
Comment #7
elachlan CreditAttribution: elachlan commentedAttiks could you please review the patch in the child issue #2605308: Add csslint to the testrunners?
Comment #8
elachlan CreditAttribution: elachlan commentedComment #9
LewisNymanMaybe this test could be tweaked so new patches don't add any more errors?
Comment #10
elachlan CreditAttribution: elachlan commentedIt is a separate job. So we can have it pass until all errors are removed and then we switch it to fail.
Here is the output from #1299710: [meta] Automate the coding-standards part of patch review
Comment #11
catchYes warn rather than fail, so we can have everything switched on and not blocked on core would be great.
Comment #12
joelpittetJust something I noticed, though I'm sure you're all aware:
core/vendor
is nowvendor
Comment #13
YesCT CreditAttribution: YesCT commentedComment #14
Mile23Setting to 'needs work' based on #12.
Comment #15
Mile23Addressing blockers for this issue: #2654650: [meta] Jobs for linting and coding standards
Comment #16
MixologicComment #17
MixologicIm working on this this week.
Comment #18
elachlan CreditAttribution: elachlan commented@Mixologic - Thanks for the update!
Comment #19
MixologicOkay, so it turns out the the .csslintrc file that comes with core, which is what we want to use as a fallback if there isnt anything defined for a project, is broken.
This is blocked on fixing core: #2799603: .csslintrc file format is invalid and results in "important is not a valid option"
Comment #20
elachlan CreditAttribution: elachlan commentedComment #21
MixologicComment #23
hestenetThe core issue is now fixed - un-postponing this one.
Comment #24
MixologicThis has been deployed to production. We have csslint checkstyle results now.
Comment #25
Mixologic