#38 Fixed watch & browsersync gulp tasks. by hog-seruj · Pull Request #39 · skilld-labs/zen

hog-seruj · 2017-09-22T15:46:53Z

No description provided.

andriyun · 2017-09-24T20:29:39Z

 ];

-gulp.task('styles', ['clean:css'], function () {
+gulp.task('styles', function () {


What the reason to removing clean:css task here?

andriyun · 2017-09-24T20:30:49Z

+var watchCss = gulp.watch(sassFiles, options.gulpWatchOptions, ['styles']);
+gulp.task('watch:css', function () {
+  gulp.watch(options.theme.css + '**/*.css').on('change', browserSync.reload);
+  return watchCss


Here should be ;

iberdinsky-skilld · 2017-11-25T19:11:32Z

Npm tasks

Dist. Clean css, js, maps, styleugude. Compact css, compact js. Syleguide.
Watch. Clean css, js, maps. Mapped css, mapped js, Styleguide.
Serve. Clean css, js, maps. Mapped css, mapped js, Styleguide.

Gulp.

build. default tasks
watch, devtask with watcher
serve, devtask with watcher and browsersync

Updates

Variables in begin, Functions on bottom.
Added pattern * for gulp-load-plugins to use all package.json modules with $.MODULE_NAME They will loded when they needed.
Added xip by default to browsersync options. http://xip.io/
Added npm run tasks. Foe easier docker frontend using.
Fixed watch and browsersync tasks.
Removed filenames log from gulp:sass task. Who need it?
Changed "no-empty-rulesets" sass linter rule to "info" instead of warning. Or we need to fix that empty rules in default styles.

andypost

Moving code around makes review harder

andypost · 2017-11-26T14:18:16Z

 });

+// Converts module names to absolute paths for easy imports.
+function sassModuleImporter(url, file, done) {


Please do not move code around the file, edit it where it lives

structure will change more. actually i think this function will be changed with some module.

andypost · 2017-11-26T14:18:53Z

    "browser-sync": "^2.12.3",
    "chroma-sass": "^1.2.4",
    "del": "^2.2.0",
-    "eslint": "^2.9.0",


why eslint removed?

https://www.npmjs.com/package/gulp-eslint we have this. otherwice conflict in load plugins.

pabloguerino · 2017-11-27T15:38:25Z

  no-debug: 2
  # no-duplicate-properties: 2
-  no-empty-rulesets: 2
+  no-empty-rulesets: 1


Why setting it to a warning instead of an error? No need to keep empty rules.

https://github.com/sasstools/sass-lint/blob/master/docs/rules/no-empty-rulesets.md

pabloguerino · 2017-11-27T15:40:02Z

-  cache = require('gulp-cached'),
-  spritesmith = require('gulp.spritesmith');
+// Browsersync options
+options.browserSync = {


Maybe better to have this one in a separated config file like we do on webpack?

yep. in future plans.
#44
also we need to move config.

hog-seruj · 2017-11-29T11:06:51Z

'Watch' task work for scss files. Bu i see some issues:

Need add default image for sprite generation
After watch all project recompile, it's very long. Need think togather how to fix it. In my mind, only file we change must be recompiled
Watch steel not work for js

iberdinsky-skilld · 2017-11-29T11:33:23Z

@hog-seruj
1. Need add default image for sprite generation 2. After watch all project recompile, it's very long. Need think togather how to fix it. In my mind, only file we change must be recompiled 3. Watch steel not work for js

yeah. it was here. could you revert if you have time?
to optimize watch task we need optimize current sass structure.
Yeah. Only linter. We need to apply webpack build and reload after.

#38 Fixed watch & browsersync gulp tasks.

04321ae

andriyun reviewed Sep 24, 2017

View reviewed changes

iberdinsky-skilld self-assigned this Nov 22, 2017

Fixed watch/browsersync.

3b8bb1a

iberdinsky-skilld requested a review from andypost November 25, 2017 19:23

andypost requested changes Nov 26, 2017

View reviewed changes

finnsky added 2 commits November 26, 2017 22:58

Merge remote-tracking branch 'origin' into f38

7033b4b

Move funcion to its original place.

c7ce8ca

pabloguerino suggested changes Nov 27, 2017

View reviewed changes

Conversation

hog-seruj commented Sep 22, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iberdinsky-skilld commented Nov 25, 2017

Npm tasks

Gulp.

Updates

Uh oh!

andypost left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hog-seruj commented Nov 29, 2017

Uh oh!

iberdinsky-skilld commented Nov 29, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants