General FE Workflow [DO NOT MERGE] #245
Conversation
…asks: `clean`, `css`, `css-lint`).
|
Should I merge this?
|
|
Only if you're feeling lucky :) |
…flow-update * feature/front-end-workflow: Updated devDependencies Update `package.json`. Add support for `postcss-assets` processor. Improve PostCSS processors handling. # Conflicts: # package.json # workflows/tasks/css.js
…mpletion signalling.
mehigh
changed the title
|
Referencing koop's PR: #246 |
- Make `prod` env work. - Split the main Gulp file into smaller modules. - Add first Jest tests. - Rely on Node version setting from the `package.json` instead of hardcoded value.
…nd-workflow-update-recs
Recommendations for workflow updates.
…-dev-lib into feature/front-end-workflow-update * 'feature/front-end-workflow-update' of github.com:xwp/wp-dev-lib: Set rules as null to not enforce. Update quotes for JSON. Recommendations for workflow updates.
package.json
Outdated
| } | ||
| ], | ||
| "selector-pseudo-element-colon-notation": "double", | ||
| "value-keyword-case": null |
There was a problem hiding this comment.
Why are the WordPress CSS Coding Standards being overridden here?
(For clarification, there are 8 rules being overridden, not easily reviewable in the PR review view)
There was a problem hiding this comment.
See also the stylelint pull request #239 where the proposal is to use styelint-config-wordpress as is without overriding any rules
There was a problem hiding this comment.
@ntwb Thank you for your comment.
First of all, please note that this ruleset is still a work in progress and may change.
To answer your question - we decided to relax or override some of the rules defined in the stylelint-config-wordpress/scss based on the way we usually format our SCSS code.
In some points our best practices align quite well with the CSS rules set by Automattic in the Calypso project:
- We prefer using single quotes (
string-quotes). It reflects the way strings in PHP and JS are treated in WordPress. - We always use quotes, in
urlfunctions too (function-url-quotes). - We allow whitespace inside parentheses (
function-parentheses-space-inside). It, again, better reflects the way we format PHP and JS code. - We are not requiring the hex color values to be short whenever possible (
color-hex-length), since they will get optimized/shortened in the SCSS compilation process.
There are also grey areas, not defined in the WordPress CSS Coding Standards, e.g.:
- Since the standards describe CSS and not SCSS code, we decided to set the
at-rule-empty-line-beforerule in this particular way. We feel that the code is easier to read with the such setup. - The standards don't mention the
selector-pseudo-element-colon-notation. Our guess is that the requirement for a single colon is included in thestylelint-config-wordpressto maintain the Internet Explorer 8 support. Since IE 8 is no longer supported by WordPress as of version 4.8, we feel our code should use the valid CSS notation, i.e. double colons.
When it comes to the value-keyword-case my opinion is that is should not be overridden here. We should always use lowercase for CSS values. @mehigh and @kopepasah what do you think about it?
There was a problem hiding this comment.
I agree value-keyword-case should be enforced to lower.
There was a problem hiding this comment.
Where we diverge from core standards, should we not create a separate ruleset like stylelint-ruleset-xwp that inherits from stylelint-ruleset-wordpress? Then wp-dev-lib can continue to just use stylelint-ruleset-wordpress and our projects can override the package.json to specify our flavour. Otherwise, we should try to upstream our changes if they are compelling.
There was a problem hiding this comment.
Thanks for the detailed explanation @delawski, I now see what I was missing here was a bit of context that @westonruter has now clarified. As is obvious that this repo is a XWP project, I also know quite a few people outside of XWP use this repo(or least parts of it) so diverging from WordPress' Coding Standards had me a little perplexed.
Going the route of creating stylelint-config-xwp would be 👌
Your guess in regard to selector-pseudo-element-colon-notation is correct, it was because of IE8, I've created WordPress-Coding-Standards/stylelint-config-wordpress#165 to update to double colons 👍
There was a problem hiding this comment.
This sounds really good. I went ahead and created a stylelint-config-xwp based on the stylelint-config-wordpress.
I will update the wp-dev-lib to use stylelint-config-wordpress while on most non-Core projects we will use stylelint-config-xwp.
package.json
Outdated
| "postcss-scss": "^1.0.2", | ||
| "progress-bar-webpack-plugin": "^1.10.0", | ||
| "require-dir": "^0.3.2", | ||
| "stylelint": "^7.13.0", |
There was a problem hiding this comment.
This should be ^8.0.0 as stylelint-config-wordpress's 12.0.0 requires stylelint 8.0.0
There was a problem hiding this comment.
Good catch! Thank you. We will update the stylelint version here.
|
Closing this as all FE developments have moved to a separate repository focused solely on FE build: |
No description provided.