This repository was archived by the owner on Nov 10, 2022. It is now read-only.
General FE Workflow [DO NOT MERGE] #245
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| **/*.min.js | ||
| **/node_modules/** | ||
| **/vendor/** | ||
| **/dist/** |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,2 @@ | ||
| codecept.phar | ||
| node_modules |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| #!/bin/bash | ||
|
|
||
| set -e | ||
|
|
||
| rm -rf ./workflows/dist | ||
| ./node_modules/.bin/babel ./workflows/src --out-dir ./workflows/dist | ||
| mv ./workflows/dist/gulpfile.babel.js ./workflows/dist/gulpfile.js |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| 'use strict'; | ||
|
|
||
| var _gulp = require('gulp'); | ||
|
|
||
| var _gulp2 = _interopRequireDefault(_gulp); | ||
|
|
||
| var _preCheck = require('./utils/pre-check'); | ||
|
|
||
| var _getTasks = require('./utils/get-tasks'); | ||
|
|
||
| function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; } | ||
|
|
||
| // Check Node version and workflow setup. | ||
| (0, _preCheck.preCheck)(); | ||
|
|
||
| // Define default task. | ||
| _gulp2.default.task('default', _gulp2.default.series((0, _getTasks.getTasks)())); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| 'use strict'; | ||
|
|
||
| var _gulp = require('gulp'); | ||
|
|
||
| var _gulp2 = _interopRequireDefault(_gulp); | ||
|
|
||
| var _getConfig = require('../utils/get-config'); | ||
|
|
||
| var _del = require('del'); | ||
|
|
||
| var _del2 = _interopRequireDefault(_del); | ||
|
|
||
| var _TaskHelper = require('../utils/TaskHelper'); | ||
|
|
||
| var _TaskHelper2 = _interopRequireDefault(_TaskHelper); | ||
|
|
||
| function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; } | ||
|
|
||
| var task = new _TaskHelper2.default({ | ||
| name: 'clean', | ||
| requiredPaths: ['src'], | ||
| config: _getConfig.tasks | ||
| }); | ||
|
|
||
| _gulp2.default.task(task.name, function (done) { | ||
| if (task.isValid()) { | ||
| (0, _del2.default)(task.src).then(function () { | ||
| return done(); | ||
| }); | ||
| } | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| 'use strict'; | ||
|
|
||
| var _gulp = require('gulp'); | ||
|
|
||
| var _gulp2 = _interopRequireDefault(_gulp); | ||
|
|
||
| var _gulpIf = require('gulp-if'); | ||
|
|
||
| var _gulpIf2 = _interopRequireDefault(_gulpIf); | ||
|
|
||
| var _gulpCached = require('gulp-cached'); | ||
|
|
||
| var _gulpCached2 = _interopRequireDefault(_gulpCached); | ||
|
|
||
| var _getConfig = require('../utils/get-config'); | ||
|
|
||
| var _TaskHelper = require('../utils/TaskHelper'); | ||
|
|
||
| var _TaskHelper2 = _interopRequireDefault(_TaskHelper); | ||
|
|
||
| function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; } | ||
|
|
||
| var task = new _TaskHelper2.default({ | ||
| name: 'copy', | ||
| requiredPaths: ['src', 'dest'], | ||
| config: _getConfig.tasks | ||
| }); | ||
|
|
||
| _gulp2.default.task(task.name, function () { | ||
| if (!task.isValid()) { | ||
| return null; | ||
| } | ||
|
|
||
| return task.start().pipe((0, _gulpIf2.default)(_getConfig.isDev, (0, _gulpCached2.default)(task.cacheName, { optimizeMemory: false }))).pipe(task.end()); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| 'use strict'; | ||
|
|
||
| var _gulp = require('gulp'); | ||
|
|
||
| var _gulp2 = _interopRequireDefault(_gulp); | ||
|
|
||
| var _gulpCached = require('gulp-cached'); | ||
|
|
||
| var _gulpCached2 = _interopRequireDefault(_gulpCached); | ||
|
|
||
| var _getConfig = require('../utils/get-config'); | ||
|
|
||
| var _gulpIf = require('gulp-if'); | ||
|
|
||
| var _gulpIf2 = _interopRequireDefault(_gulpIf); | ||
|
|
||
| var _gulpPostcss = require('gulp-postcss'); | ||
|
|
||
| var _gulpPostcss2 = _interopRequireDefault(_gulpPostcss); | ||
|
|
||
| var _postcssReporter = require('postcss-reporter'); | ||
|
|
||
| var _postcssReporter2 = _interopRequireDefault(_postcssReporter); | ||
|
|
||
| var _postcssScss = require('postcss-scss'); | ||
|
|
||
| var _postcssScss2 = _interopRequireDefault(_postcssScss); | ||
|
|
||
| var _stylelint = require('stylelint'); | ||
|
|
||
| var _stylelint2 = _interopRequireDefault(_stylelint); | ||
|
|
||
| var _TaskHelper = require('../utils/TaskHelper'); | ||
|
|
||
| var _TaskHelper2 = _interopRequireDefault(_TaskHelper); | ||
|
|
||
| function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; } | ||
|
|
||
| var task = new _TaskHelper2.default({ | ||
| name: 'css-lint', | ||
| requiredPaths: ['src'], | ||
| config: _getConfig.tasks, | ||
| configSlug: 'css' | ||
| }); | ||
|
|
||
| if (undefined !== task.config) { | ||
| _gulp2.default.task(task.name, function () { | ||
| if (!task.isValid()) { | ||
| return null; | ||
| } | ||
|
|
||
| return task.start().pipe((0, _gulpIf2.default)(_getConfig.isDev, (0, _gulpCached2.default)(task.cacheName))).pipe((0, _gulpPostcss2.default)([(0, _stylelint2.default)(), (0, _postcssReporter2.default)({ clearAllMessages: true })], { syntax: _postcssScss2.default })); | ||
| }); | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See also the stylelint pull request #239 where the proposal is to use
styelint-config-wordpressas is without overriding any rulesThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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/scssbased 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:
string-quotes). It reflects the way strings in PHP and JS are treated in WordPress.urlfunctions too (function-url-quotes).function-parentheses-space-inside). It, again, better reflects the way we format PHP and JS code.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.:
at-rule-empty-line-beforerule in this particular way. We feel that the code is easier to read with the such setup.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-casemy 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree
value-keyword-caseshould be enforced to lower.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where we diverge from core standards, should we not create a separate ruleset like
stylelint-ruleset-xwpthat inherits fromstylelint-ruleset-wordpress? Then wp-dev-lib can continue to just usestylelint-ruleset-wordpressand our projects can override thepackage.jsonto specify our flavour. Otherwise, we should try to upstream our changes if they are compelling.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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-xwpwould be 👌Your guess in regard to
selector-pseudo-element-colon-notationis 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sounds really good. I went ahead and created a
stylelint-config-xwpbased on thestylelint-config-wordpress.I will update the
wp-dev-libto usestylelint-config-wordpresswhile on most non-Core projects we will usestylelint-config-xwp.