Problem/Motivation

Follow-up to #2865971: Use stylelint as opposed to csslint in core. Configure the function-comma-space-after to be consistent with https://www.drupal.org/docs/develop/standards/css/css-coding-standards

From the CSS Formatting Guidelines:

Include a space after each comma in comma-separated property or function values.

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

CommentFileSizeAuthor
#12 2866810-12.patch13.71 KBharsha012
#9 stylelint-function-comma-space-after-2866810-8.patch10.48 KBbrightbold
#4 stylelint-function-comma-space-after-incl-2865971-41-patch-2866810-4.patch50.61 KBbrightbold
#3 stylelint-function-comma-space-after-2866810-3.patch10.52 KBbrightbold

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Parent issue: #2866809: Update stylelint rule declaration-colon-space-after to be consistent with Drupal's CSS standards » #2865971: Use stylelint as opposed to csslint in core
brightbold’s picture

brightbold
she/her
Primary language English
Location Boston, MA
commented
StatusFileSize
new stylelint-function-comma-space-after-2866810-3.patch10.52 KB

function-comma-space-after set to true and CSS files updated to pass.

This patch depends on #2865971-41: Use stylelint as opposed to csslint in core and as such will fail tests until that's committed.

brightbold’s picture

brightbold
she/her
Primary language English
Location Boston, MA
commented
StatusFileSize
new stylelint-function-comma-space-after-incl-2865971-41-patch-2866810-4.patch50.61 KB

This patch is #4 above combined with #2865971-41: Use stylelint as opposed to csslint in core for testing purposes.

brightbold’s picture

brightbold
she/her
Primary language English
Location Boston, MA
commented
Status: Active » Needs review
brightbold’s picture

brightbold
she/her
Primary language English
Location Boston, MA
commented
Issue summary: View changes
brightbold’s picture

brightbold
she/her
Primary language English
Location Boston, MA
commented
Version: 8.3.x-dev » 8.4.x-dev
Assigned: Unassigned » brightbold

The last submitted patch, 3: stylelint-function-comma-space-after-2866810-3.patch, failed testing. View results

brightbold’s picture

brightbold
she/her
Primary language English
Location Boston, MA
commented
Assigned: brightbold » Unassigned
StatusFileSize
new stylelint-function-comma-space-after-2866810-8.patch10.48 KB

Rerolled.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs review » Needs work
Issue tags: +Needs reroll

This may need a reroll, minor but one of the files changed in outside_in.

error: core/modules/outside_in/css/outside_in.theme.css: does not exist in index

harsha012’s picture

harsha012 commented
Status: Needs work » Needs review
StatusFileSize
new 2866810-12.patch13.71 KB

re-rolled the patch

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs reroll

Thanks @BrightBold, @harsha012 and @alexpott!

I've applied and run the linter with and without the changes to ensure they are caught.
Then ran git diff -w to make sure all the changes to css were whitespace and a quick scan to see if anything snuck in. It's all good!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Thanks for the patch and reviews. Creditting @joelpittet for the git diff -w check.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Committed 0013e90 and pushed to 8.5.x. Thanks!

  • alexpott committed 0013e90 on 8.5.x 
    Issue #2866810 by BrightBold, harsha012, joelpittet: Update stylelint...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.