Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce gulp #14

Merged
merged 38 commits into from
Oct 12, 2018
Merged

Introduce gulp #14

merged 38 commits into from
Oct 12, 2018

Conversation

thumbnail
Copy link
Member

@thumbnail thumbnail commented Oct 5, 2018

Fixes #3, also closes #4

  • add css task
    • converts scss, autoprefixes, concatinates and minifies into 1 css file
  • add js task
    • polyfils, babelifies, concatinates and uglifies into 1 js file
    • particleJS doesn't like the 'use strict'-preset; fix this
    • svg.easing.js has broken syntax, babel can't deal with it (missing var somewhere)
  • add serve task
    • run assets tasks on watch
    • start livereload
    • start webserver
  • move all dependencies to package.json
  • add build task that just puts all the files in in one folder

Results

the site is at best about 150ms faster. it doesn't matter a lot though. It was mostly practise and wider support for es6 that #3 addressed.

@thumbnail thumbnail self-assigned this Oct 5, 2018
@thumbnail thumbnail mentioned this pull request Oct 5, 2018
2 tasks
gulpfile.js Outdated

exports.css = (cb) => {
gulp.src('_css/**/*.?(s)css')
.pipe(sourcemaps.init())
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

convert to pump

gulpfile.js Outdated
gulp.dest('dist')], cb);
};

exports.js = (cb) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't it be nicer to use one export

exports.build = series(
  clean,
  parallel(
    cssTranspile,
    series(jsTranspile, jsBundle)
  ),
  parallel(cssMinify, jsMinify),
  publish
);

Copy link
Member Author

@thumbnail thumbnail Oct 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As described in the description of the PR there's still a build-task to be made, which combines both.

I'd like the css and js tasks to be available independent of eachother anyway.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if you export everything at ones they should be available as their own tasks using gulp [task name]

@thumbnail thumbnail changed the title [EXPERIMENTAL] Introduce gulp Introduce gulp Oct 8, 2018
@thumbnail thumbnail changed the title Introduce gulp [WIP] Introduce gulp Oct 8, 2018
@delta-leonis delta-leonis deleted a comment Oct 9, 2018
@thumbnail thumbnail changed the title [WIP] Introduce gulp Introduce gulp Oct 9, 2018
@hampterhub
Copy link
Contributor

let jekyll = spawn('jekyll', ['serve', '--livereload'], { stdio: 'inherit' });

wouldn't it be to use gulp-watch, then you can also set the server parameters.

@thumbnail
Copy link
Member Author

you can pass configuration using the _config.yml file in the root. less convenient than a argument but i don't feel like supporting arguments directly to be honest.

@thumbnail thumbnail mentioned this pull request Oct 9, 2018
@xvilo
Copy link

xvilo commented Oct 10, 2018

what about:

"scripts": {
  "gulp": "node ./node_modules/gulp/bin/gulp.js <task-name>" 
}

Then: $ npm run gulp

command: gem install jekyll
command: |
gem install jekyll
sudo npm install --global
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just do npm install. Thats all you need.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you can see in the builds, such as build 67 it didn't appear to work. I tried debugging using ssh on the machine and only with all three npm installs it appeared to install gulp-cli

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So what you do, is create the npm command I commented, then just require gulp 3.x and then change line 21 to npm run gulp

Copy link
Contributor

@hampterhub hampterhub Oct 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xvilo i refactored your solution into #18 thanks for the advice.

i also learned that when using npm run there is no need to use a full path like node ./node_modules/gulp/bin/gulp.js <task-name> since npm run will add the packages to the path when you use it.

"gulp-autoprefixer": "^6.0.0",
"gulp-babel": "^8.0.0",
"gulp-clean-css": "^3.10.0",
"gulp-cli": "^2.0.1",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It didn't seem to work without, but since i explicitly install it in the circleci config I might be able to delete it. also more people suggest gulp-cli is needed

@thumbnail
Copy link
Member Author

regarding the scripts-config; intresting construction, not sure what it brings me though? could you elaborate?

@xvilo
Copy link

xvilo commented Oct 10, 2018

Then you don't have to install a lot of unrelated packages, or install things globally in the CI (use containers with the specific tools already pre installed). You can ofcourse run the commands from your ci config file. But why not add it as a nice npm run 'command'. It was just another solution to the problem. There are meerdere wegen to Rome

thumbnail and others added 2 commits October 12, 2018 16:01
* Removing things until it stops braking

* removed first npm install

* added npm gulp task, removed installation of gulp_cli

npm run gulp
=================
npm run adds node_modules to the path, $arg holds any arguments sent with gulp.

* removed `sudo`, changed script run command.
Copy link
Contributor

@hampterhub hampterhub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

@thumbnail thumbnail merged commit 643769e into master Oct 12, 2018
@thumbnail thumbnail deleted the gulp branch October 12, 2018 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Microsoft edge support add assets pipeline
3 participants