Problem/Motivation

Follow-up to #2865971: Use stylelint as opposed to csslint in core. Configure the declaration-block-trailing-semicolon to be consistent with https://www.drupal.org/docs/develop/standards/css/css-coding-standards

From the CSS Formatting Guidelines:

Include a semi-colon at the end of all declarations, including the last declaration in a declaration block.

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

Remove the stylelint rule from the core/.stylelintrc file, since the rule now matches the stylelint-config-standard that is extended, see https://github.com/stylelint/stylelint-config-standard/blob/master/index.js

Remaining tasks

User interface changes

None

API changes

None

Comments

alexpott created an issue. See original summary.

BrightBold’s picture

Issue summary: View changes
BrightBold’s picture

Set declaration-block-trailing-semicolon to "always" and added missing semicolons in css.

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

BrightBold’s picture

This patch includes the same changes as #4 but also incorporates the contents of #2865971-41: Use stylelint as opposed to csslint in core and thus can be used to run tests.

BrightBold’s picture

Status: Active » Needs review
idebr’s picture

Issue summary: View changes
Status: Needs review » Needs work
Issue tags: +Novice

Updated the issue summary to reflect the rule can be removed from the .stylelintrc, since it matches the stylelint-config-standard that is extended.

Added the Novice tag, since there is a clear issue summary and proposed resolution.

idebr’s picture

Issue summary: View changes
pk188’s picture

Status: Needs review » Needs work
pk188’s picture

Status: Needs review » Needs work
pk188’s picture

Status: Needs work » Fixed
idebr’s picture

Version: 8.3.x-dev » 8.4.x-dev
Status: Fixed » Needs work

Hi pk188,

Thanks for working on this issue!

Your patch did not apply correctly, since the target branch was misconfigured to 8.3.x instead of 8.4.x. I have updated the issue accordingly.

idebr’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work
BrightBold’s picture

Assigned: Unassigned » BrightBold
BrightBold’s picture

Assigned: BrightBold » Unassigned
BrightBold’s picture

The only differences I see between the patches in #9/#11 and the patch in #4 are changes not covered by this issue: the deletion of a commented out property and a change in the minimum browser versions. While those both may be changes we want to make, we shouldn't be making them here. So I've fix the patch from #4 to take @idebr's comment into account.