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.The file format of csslintrc file has changed - we need to adjust.
This was originally uncovered by Mile23 in
https://www.drupal.org/node/2787663#comment-11620153
in short the current .csslintrc file format works for version v0.10.0 BUT not for the latest version v1.0.2
we need to convert to a file format as demonstrated by
https://github.com/holidayextras/culture/blob/master/.csslintrc
--errors=bulletproof-font-face,display-property-grouping,duplicate-background-images,duplicate-properties,empty-rules,floats,outline-none,font-faces,ids,import,important,known-properties,qualified-headings,regex-selectors,shorthand,star-property-hack,text-indent,underscore-property-hack,unique-headings,vendor-prefix,zero-units
--warnings=compatible-vendor-prefixes,fallback-colors,font-sizes,gradients,non-link-hover,overqualified-elements
--ignore=adjoining-classes,box-model,box-sizing,unqualified-attributes,universal-selector
| Comment | File | Size | Author |
|---|---|---|---|
| #24 | Screen Shot 2017-03-07 at 3.05.04 PM.png | 122.59 KB | frob |
| #15 | csslintrc_file_format-2799603-15.patch | 2.12 KB | Mixologic |
| #6 | interdiff_4-6.txt | 1.2 KB | josephdpurcell |
| #6 | csslintrc-2799603-5.patch | 1.64 KB | josephdpurcell |
| #4 | csslintrc-2799603-3.patch | 1.6 KB | Mukeysh |
Comments
Comment #2
Mukeysh CreditAttribution: Mukeysh as a volunteer and at gai Technologies Pvt Ltd for gai Technologies Pvt Ltd commentedi have changed .csslintrc file as mentioned above. Attached patch for this
Comment #3
Mukeysh CreditAttribution: Mukeysh as a volunteer and at gai Technologies Pvt Ltd for gai Technologies Pvt Ltd commentedComment #4
Mukeysh CreditAttribution: Mukeysh as a volunteer and at gai Technologies Pvt Ltd for gai Technologies Pvt Ltd commentedPlease ignore above patch. Attached new patch for this.
Comment #5
martin107 CreditAttribution: martin107 commentedThanks Mukeysh,
I tried creating a patch for this and failed it mostly because the documentation online is poor. -- thank you for taking the time to work it out.
Mukeysh++
This patch is difficult to review and could potentially be harmful - if we get the conversion wrong or miss an element out then it is not immediately obvious.
So I have taken the time to list the before and after and go through them line by line
here is what I can see
deleted ( found in the original but in the new section.)
selector-max-approaching
rules-count
selector-max
Inserted ( not in the original but found in the new section )
non-link-hover
Comment #6
josephdpurcell CreditAttribution: josephdpurcell at Digital Bridge Solutions commentedThis issue is relevant to all Drupal 8 versions as it was committed in 8.0, see #2222049: Add a .csslintrc file that's in line with our CSS standards.
Using csslint v1.0.3 I am unable to use the .csslint that is currently in core. When I do, here is the error I get:
After applying csslintrc-2799603-3.patch I am able to use the .csslint.
I compared the list as well and here are the differences I see:
Added as error: (i.e. previously was ignored, now is an error)
bulletproof-font-face
Added as warning: (i.e. previously was ignored, now is a warning)
compatible-vendor-prefixes
fallback-colors
floats
font-faces
font-sizes
gradients
regex-selectors
Added as a warning: (i.e. did not previously exist and is now a warning)
non-link-hover
Ignored: (i.e. previously existed as an error/warning and is now ignored
box-model
unqualified-attributes
I've attached a patch that introduces (from what I understand) no change in configuration.
I agree that any change in standards could be harmful, and I don't know the CSS standards well enough to know whether this patch is accurate. Perhaps someone very familiar with CSS coding standards could review?
Comment #7
josephdpurcell CreditAttribution: josephdpurcell at Digital Bridge Solutions commentedComment #8
josephdpurcell CreditAttribution: josephdpurcell at Digital Bridge Solutions commentedTangentially related, would it be more valuable to have SASS and Less linting? Many people use SASS and Less for themeing instead of writing CSS directly.
Comment #10
joelpittetTestbot failure, resetting status due to https://www.drupal.org/node/2828045
Comment #11
xjmJust came across this issue when I was trying to check coding standards on #2793849: Handle offcanvas differently at lower widths.
Tagging for subsystem maintainer review since this defines some of our standards for CSS. Thanks for working on this!
Comment #12
xjmComment #14
MixologicIm trying to add csslint to the testrunners, and this is also blocking that.
Comment #15
MixologicThe patch in #6 had a typo -
gbox-sizingwhen it should have beenbox-sizing, so I went ahead and rerolled it.I have made three further changes:
1. I split the config file up into multiple lines, and tested it with 1.0.4 to verify that this works. This will make any further changes to this file much, much easier to review.
2. I have added the following three rules that are defined in 1.0.4, and added them to the --ignore section - they will be executed if they are not explicitly ignored, and I would like to preserve the current configuration state.
3. I have added an
--exclude-listsection to prevent code sniffing of /vendor or /core/assets, as there isnt much sense in checking other peoples coding standards.Comment #16
elachlan CreditAttribution: elachlan commentedComment #17
frobI will take a look at reviewing this on Monday.
Comment #18
frobAs I understand it this issue is about getting a working ruleset for csslint. As far as that goes it works. I was able to apply the patch and run it against seven's print.css
It came back with 22 errors, but the rules parsed.
The summary is here https://travis-ci.org/frob/drupal-test/builds/205466622
Comment #19
alexpottThis doesn't change any of the rules already defined it just fixes the format of the file to work with the latest version of the linting tool. Given that I don't think that we need to wait on sub-system maintainer review as they signed off on these rules in #2222049: Add a .csslintrc file that's in line with our CSS standards
Committed and pushed 21f8a65 to 8.4.x and 16c1620 to 8.3.x. Thanks!
Comment #22
frobSince this is a testbot issue, should we apply this patch to 8.2, 8.1 and 8.0? Patches can still be told to test against those older versions.
Comment #23
alexpott@frob we don't make changes to those branches 8.2.x (and the previous ones) will not receive another release. No core patch will every be applied from them so there is no point.
Comment #24
frob@alexpott, We should probably open a new issue, currently there are many ways to run the testbot on old versions of 8. For example, it I setup the auto testing on a contrib module.
Any module that has been setup to receive automatic tests on an old version will forever fail if csslint is turned on and set as a failing condition. I understand that those old releases are no longer supported, I was just pointing out that this will happen.
Comment #25
Mile23CSS linting (and Coder sniffing) can be set up per project, so your contrib module can have its own .csslintrc. http://cgit.drupalcode.org/drupalci_testbot/tree/src/DrupalCI/Plugin/Bui...
If there's no .csslintrc then the testbot won't do a CSS lint at all.
So that leaves three solutions: 1) Fix .csslintrc in old Drupal versions, 2) Remove .csslintrc from old Drupal versions, 3) Always make sure your contrib project's .csslintrc is up-to-date if you plan to test it against non-release versions of Drupal core.
Looks like 1 and 2 are probably not going to happen.
Comment #26
Mixologic@frob: we do have an issue: #2856121: Testing on d.o. should no longer offer outdated versions of tests and should change testing configuration when versions move
There really isn't any need to test against older versions of core, and 8.2.x is only going to be something we care about for a little while longer.
Since we're not currently failing test status on anything on css, only reporting failures, its probably not that big of a deal. Most contrib module tests that Im seeing in the database are using 8.3.x or 8.4.x as a general rule.