Closed (fixed)
Project:
Drupal core
Version:
8.6.x-dev
Component:
CSS
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
4 Apr 2017 at 19:38 UTC
Updated:
21 Feb 2018 at 08:59 UTC
Jump to comment: Most recent, Most recent file
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-sourcerule totrue.This patch is dependent on #2865971-41: Use stylelint as opposed to csslint in core.
Comment #4
brightboldComment #6
harsha012 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
jofitzYou know I'm always happy to provide a re-roll.
Comment #10
jofitzComment #11
harsha012 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 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 commentedfixed as per #16
Comment #18
harsha012 commentedComment #19
harsha012 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
jofitzAdd 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.