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

Ajout de l'option --speed pour gulp #5060

Merged
merged 1 commit into from
Dec 5, 2018
Merged

Conversation

A-312
Copy link
Contributor

@A-312 A-312 commented Sep 5, 2018

Corrige : #5058

La commande --speed :

  • Évite la compression des images ;
  • Et évite uglify du .JS.

NB : Je me suis permis de corriger certaine erreur d'indentation mais je crois que c'est quelque chose à ne pas faire ?

Q/A :

Faire gulp clean avant puis gulp watch --speed.

(Pensez à faire npm install ou npm install gulp-options)

EDIT : Remplacement de fast par speed.

@coveralls
Copy link

coveralls commented Sep 5, 2018

Coverage Status

Coverage remained the same at 89.249% when pulling 6f1ac55 on A-312:testgulp into d84501a on zestedesavoir:dev.

@A-312
Copy link
Contributor Author

A-312 commented Sep 5, 2018

Je ne comprends pas les erreurs des bots/travis, un problème de syntaxe ?

@A-312
Copy link
Contributor Author

A-312 commented Sep 5, 2018

Q/A : Pas besoin d'essayer ça ne fonctionne pas.

EDIT : gulp build me retourne une erreur sans --fast, j'ai du casser quelques choses. :(

EDIT2 : J'ai peut être fait l'erreur de mettre à jour npm. Pas ça.

EDIT3: Je ne sais pas : gulp js ne se termine pas.

$ gulp js
[19:28:15] Using gulpfile ~/zds/zds-site/Gulpfile.js
[19:28:15] Starting 'js'...
$

@Situphen
Copy link
Member

Situphen commented Sep 5, 2018

Le gain de temps entre lancement de la commande avec ou sans l'option est vraiment flagrant ?

En soit je suis pas contre, par contre on ne met pas l'option pour Travis CI car il faut que la compression des images et la minification du JS soient testées quelque part :)

@A-312
Copy link
Contributor Author

A-312 commented Sep 5, 2018

@Situphen Je passe à quelques secondes/ms au lieu de : https://zestedesavoir.com/forums/sujet/11251/probleme-pour-watch-le-front-end/?page=2#p188382

@A-312
Copy link
Contributor Author

A-312 commented Sep 5, 2018

J'ai trouvé le problème :

Ne fonctionne pas :

    src.pipe(sourcemaps.write('.', { includeContent: true, sourceRoot: '../../' }));
    src.pipe(gulp.dest('dist/js/'));

Fonctionne :

    .pipe(sourcemaps.write('.', { includeContent: true, sourceRoot: '../../' }))
    .pipe(gulp.dest('dist/js/'));

Donc faire src = src.pipe; et déclarer avec let.

@A-312
Copy link
Contributor Author

A-312 commented Sep 5, 2018

--fast comprend des niveaux selon votre niveau de patience xD,

LEVEL --fast=

  1. Toutes les optimisations (par défaut ou --fast=0).
  2. Aucune sauf le CSS (reste : autoprefixer + cssnano) (--fast ou --fast=1) ;
  3. Aucune sauf autoprefixer pour CSS (reste : autoprefixer) (--fast=2) ;
  4. Aucune optimisation (--fast=3).

Copy link
Contributor

@sandhose sandhose left a comment

Choose a reason for hiding this comment

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

Je pense que globalement le gros du gain il est sur uglify (et encore ?) et imagemin. Le reste je suis pas sûr que ça soit nécessaire (surtout que ça complexifie pas mal le tout)

Gulpfile.js Outdated Show resolved Hide resolved
Gulpfile.js Outdated Show resolved Hide resolved
Gulpfile.js Outdated Show resolved Hide resolved
Gulpfile.js Show resolved Hide resolved
Gulpfile.js Show resolved Hide resolved
Gulpfile.js Outdated Show resolved Hide resolved
@vhf
Copy link
Contributor

vhf commented Sep 6, 2018

Il faut changer la logique de ces arguments, ou renommer fast en slow. Ah non, en fait quand tu dis "optimisation" ça veut dire "pas optimisation". Ok, c'est tordu.

@A-312
Copy link
Contributor Author

A-312 commented Sep 6, 2018

@vhf : Ce n'est pas mon code que je considère comme optimisation mais les commandes que je désactive

Gulpfile.js Show resolved Hide resolved
Copy link
Contributor Author

@A-312 A-312 left a comment

Choose a reason for hiding this comment

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

:-)

NB : J'ai voulu tester le bouton.

Gulpfile.js Show resolved Hide resolved
@vhf
Copy link
Contributor

vhf commented Sep 8, 2018

Tu as corrigé les conditions mais pas la raison pour laquelle elles étaient fausses.

@A-312
Copy link
Contributor Author

A-312 commented Sep 8, 2018

Elles sont fausse pour la simple raison que quand je commence à utiliser >=, je m'embrouille.

Gulpfile.js Outdated Show resolved Hide resolved
@A-312
Copy link
Contributor Author

A-312 commented Sep 8, 2018

Je corrige les autres idioties présentes puis je verrais la semaine prochaine pour lire le code à tête reposée.

@artragis artragis added the C-DevelopmentEnv Amélioration de l'environnement de dev label Sep 11, 2018
@A-312
Copy link
Contributor Author

A-312 commented Sep 19, 2018

Là ça doit être bon ;)

@A-312
Copy link
Contributor Author

A-312 commented Nov 2, 2018

J'ai rebase :)

@A-312
Copy link
Contributor Author

A-312 commented Nov 7, 2018

LEVEL --speed=

  1. Toutes les fonctions de parsing pour la prod (par défaut ou --speed=0).
  2. Désactivation des fonctions de parsing du JS (uglify), des images (imagemin) sauf le CSS (--speed=1) ; --> (reste : autoprefixer + cssnano)
  3. Désactivation des précédentes [JS + IMAGES] + CSSNano. (--speed=2) ; --> (reste : autoprefixer)
  4. Désactivation de toutes les fonctions de parsing pour la prod (--speed=3 ou --speed).

(J'ai viré le terme optimisation qui portait à confusion car j'optimise la vitesse d'un code avec des fonctions qui optimisent le code pour la prod

Les fonctions que j'enlève optimisent le code pour la prod
Mais mon code optimise gulpfile pour avoir un meilleur temps).

@A-312 A-312 changed the title Ajout de l'option --fast pour gulp Ajout de l'option --speed pour gulp Nov 8, 2018
@A-312
Copy link
Contributor Author

A-312 commented Nov 8, 2018

J'ai remplacé fast par speed car c'était une variable utilisé dans gulp d'après ce qu'on m'a dit

@A-312
Copy link
Contributor Author

A-312 commented Nov 21, 2018

C'est utile que je rebase maintenant ou j'attends avant de le faire ?

@A-312
Copy link
Contributor Author

A-312 commented Nov 24, 2018

@sandhose Peux-tu regarder à nouveau ?


Pour palier au bloc de condition dans le code je pourrais utiliser ce plugin : https://github.com/robrich/gulp-if

Pour faire :

const gulpif = require('gulp-if');

gulp.task('css', ['css:sprite'], () =>
    gulp.src([
        'assets/scss/main.scss',
        'assets/scss/zmd.scss'
    ])
    .pipe(sourcemaps.init())
    .pipe(customSass())
    //    Disabled with 3 or more :
    .pipe(gulpif((speedLevel < 3), postcss(postcssPlugins)));
    .pipe(sourcemaps.write('.', {
        includeContent: true,
        sourceRoot: '../../assets/scss/'
    }))
    .pipe(gulp.dest('dist/css/'))
);

au lieu de :

gulp.task('css', ['css:sprite'], () => {
    let src = gulp.src([
            'assets/scss/main.scss',
            'assets/scss/zmd.scss'
        ])
        .pipe(sourcemaps.init())
        .pipe(customSass());

    if (speedLevel < 3) // Disabled with 3 or more
        src = src.pipe(postcss(postcssPlugins));

    src = src.pipe(sourcemaps.write('.', {
            includeContent: true,
            sourceRoot: '../../assets/scss/'
        }))
        .pipe(gulp.dest('dist/css/'));

    return src;
});

@A-312
Copy link
Contributor Author

A-312 commented Nov 28, 2018

Une idée des temps :

$ gulp build
[14:25:46] Using gulpfile ~/zds/zds-site/Gulpfile.js
[14:25:46] Starting 'prepare-zmd'...
[14:25:46] Starting 'css:sprite'...
[14:25:46] Starting 'js'...
[14:26:05] Finished 'js' after 19 s
[14:26:05] Finished 'prepare-zmd' after 19 s
[14:26:08] Finished 'css:sprite' after 22 s
[14:26:08] Starting 'css'...
[14:26:08] Starting 'images'...
[14:26:14] Finished 'css' after 6.03 s
[14:27:07] gulp-imagemin: Minified 393 images (saved 370 kB - 37.5%)
[14:27:07] Finished 'images' after 59 s
[14:27:07] Starting 'build'...
[14:27:07] Finished 'build' after 123 μs
$ gulp build --speed=1
The speed mode is enabled. LEVEL = 1
[14:34:01] Using gulpfile ~/zds/zds-site/Gulpfile.js
[14:34:01] Starting 'prepare-zmd'...
[14:34:01] Starting 'css:sprite'...
[14:34:01] Starting 'js'...
[14:34:22] Finished 'js' after 21 s
[14:34:22] Finished 'prepare-zmd' after 22 s
[14:34:25] Finished 'css:sprite' after 24 s
[14:34:25] Starting 'css'...
[14:34:25] Starting 'images'...
[14:34:25] Finished 'images' after 834 ms
[14:34:30] Finished 'css' after 5.23 s
[14:34:30] Starting 'build'...
[14:34:30] Finished 'build' after 101 μs
$ gulp build --speed=2
The speed mode is enabled. LEVEL = 2
[14:32:57] Using gulpfile ~/zds/zds-site/Gulpfile.js
[14:32:57] Starting 'prepare-zmd'...
[14:32:57] Starting 'css:sprite'...
[14:32:57] Starting 'js'...
[14:32:58] Finished 'js' after 410 ms
[14:32:58] Finished 'prepare-zmd' after 493 ms
[14:32:59] Finished 'css:sprite' after 2.01 s
[14:32:59] Starting 'css'...
[14:32:59] Starting 'images'...
[14:33:00] Finished 'images' after 785 ms
[14:33:02] Finished 'css' after 2.51 s
[14:33:02] Starting 'build'...
[14:33:02] Finished 'build' after 128 μs
$ gulp build --speed=3
The speed mode is enabled. LEVEL = 3
[14:31:50] Using gulpfile ~/zds/zds-site/Gulpfile.js
[14:31:50] Starting 'prepare-zmd'...
[14:31:50] Starting 'css:sprite'...
[14:31:50] Starting 'js'...
[14:31:51] Finished 'js' after 381 ms
[14:31:51] Finished 'prepare-zmd' after 468 ms
[14:31:52] Finished 'css:sprite' after 1.92 s
[14:31:52] Starting 'css'...
[14:31:52] Starting 'images'...
[14:31:53] Finished 'images' after 716 ms
[14:31:53] Finished 'css' after 855 ms
[14:31:53] Starting 'build'...
[14:31:53] Finished 'build' after 118 μs

@A-312
Copy link
Contributor Author

A-312 commented Nov 28, 2018

Voila c'est peut être mieux comme ça.

@A-312
Copy link
Contributor Author

A-312 commented Dec 2, 2018

@Situphen plus de niveau

@A-312
Copy link
Contributor Author

A-312 commented Dec 2, 2018

J'ai fais sauter mon commit, super >.<

@Situphen
Copy link
Member

Situphen commented Dec 2, 2018

Renomme --speed en --fast et c'est bon pour moi !

Aussi, ce serait pas mal si tu n'avais qu'un seul commit. Tu peux par exemple faire git reset HEAD~22 si tu as 22 commits pour supprimer les commits en gardant les modifications (attention car cette commande est assez dangereuse). Ensuite tu peux faire git commit -a pour refaire un commit tout proche. Et enfin, git push -f origin pour forcer l'envoi (attention là encore, commande dangereuse).

@A-312
Copy link
Contributor Author

A-312 commented Dec 2, 2018

Quelqu'un m'avait dit sur l'IRC qu'il avait des problèmes avec l'option "--speed" "--fast" car ça rentrait en confusion avec une autre commande de gulp.

EDIT : Lapsus.

@A-312 A-312 force-pushed the testgulp branch 2 times, most recently from 5e60020 to 632769c Compare December 2, 2018 19:52
@Situphen
Copy link
Member

Situphen commented Dec 2, 2018

J'ai testé npm run gulp build -- --speed et npm run gulp build et on voit bien la différence !

QA OK pour moi

@Situphen
Copy link
Member

Situphen commented Dec 4, 2018

@A-312 Il faut un petit git rebase upstream/dev et je peux merger :)

@A-312
Copy link
Contributor Author

A-312 commented Dec 5, 2018

C'est fait @Situphen

@artragis
Copy link
Member

artragis commented Dec 5, 2018

Comme situphen a approuvé, je merge.

@artragis artragis merged commit a7d0b9f into zestedesavoir:dev Dec 5, 2018
@A-312 A-312 deleted the testgulp branch December 8, 2018 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-DevelopmentEnv Amélioration de l'environnement de dev
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants