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.
Problem/Motivation
Follow-up to #2865971: Use stylelint as opposed to csslint in core. Configure the no-empty-source
to be consistent with https://www.drupal.org/docs/develop/standards/css/css-coding-standards
Proposed resolution
Brief instructions on running stylelint - you'll need npm...
All the commands below take place in DRUPAL_ROOT/core
To install stylelint
npm install
This will install Drupal 8's npm dependencies of which stylelint is one.
To run it on all core css files. Apply this issue's patch and do the following command from DRUPAL_ROOT/core
npm run lint:css
Remaining tasks
User interface changes
None
API changes
None
Comment | File | Size | Author |
---|---|---|---|
#21 | 2866817-21.patch | 865 bytes | jofitz |
#19 | 2866817-18.patch | 807 bytes | harsha012 |
#19 | 2866817-17.patch | 892 bytes | harsha012 |
#15 | 2866817-15.patch | 1.06 KB | harsha012 |
#15 | 2866817-15.patch | 1 KB | harsha012 |
Comments
Comment #2
alexpottComment #3
BrightBoldThe only code that fails this test is Stark's layout.css, which is completely empty. I wasn't sure what we wanted to do about that, so I left it alone. This tiny patch has nothing except for changing the
no-empty-source
rule totrue
.This patch is dependent on #2865971-41: Use stylelint as opposed to csslint in core.
Comment #4
BrightBoldComment #6
harsha012 CreditAttribution: harsha012 as a volunteer and at Red Crackle commentedComment #7
joelpittetThe problem with changing the same file in different patches is that there needs to be a reroll each other one that gets committed.
Comment #8
jofitz CreditAttribution: jofitz at ComputerMinds commentedYou know I'm always happy to provide a re-roll.
Comment #10
jofitz CreditAttribution: jofitz at ComputerMinds commentedComment #11
harsha012 CreditAttribution: harsha012 as a volunteer and at Red Crackle commentedfixed the patch for 8.4.x
Comment #13
joelpittetThanks @Jo Fitzgerald, @harsha012, @BrightBold, and @alexpott
I'm RTBC #8. I tested the linter with and without the CSS changes and reviewed the changes.
I'm a bit unsure why we have an empty stylesheet and half tempted to recommend removing it but I'll let a committer help decide it's fate.
Comment #14
alexpott#2349711: Remove all visual from stark removed all the CSS from this file. I think we didn't remove it for BC. I think the comment added needs to reflect why it is empty. I didn't fully read the related issue for the reason.
Comment #15
harsha012 CreditAttribution: harsha012 as a volunteer and at Red Crackle commentedfixed as per the #14 comment
Comment #16
alexpott@harsha012 thanks for the patch. I think we need to add a comment saying it is here for testing purposes - see #2349711-87: Remove all visual from stark
Also there needs to be a new line on the end of the file to comply with coding standards.
Comment #17
harsha012 CreditAttribution: harsha012 as a volunteer and at Red Crackle commentedfixed as per #16
Comment #18
harsha012 CreditAttribution: harsha012 as a volunteer and at Red Crackle commentedComment #19
harsha012 CreditAttribution: harsha012 as a volunteer and at Red Crackle commentedComment #20
joelpittet@harsha012 just need a new line and should be made against 8.6.x, it's not a bug so we don't need to backport it. And the line could probably end with "purposes."
Comment #21
jofitz CreditAttribution: jofitz at ComputerMinds commentedAdd new line.
Change "purpose" to "purposes".
Comment #22
joelpittetThanks!
Comment #23
alexpottCommitted and pushed 0a1864356c to 8.6.x and f3a5f932a0 to 8.5.x. Thanks!
Adding commit credit for reviewers.