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

Example fs-extra #100

Closed
wants to merge 5 commits into from
Closed

Conversation

DirtyF
Copy link
Collaborator

@DirtyF DirtyF commented Jan 29, 2017

Le script d'origine n'était pas très explicite lorsqu'on le lançait en ligne de commande.

L'exemple pourrait être :

  • on vérifie si le dossier de destination de la copie existe (ensure) ;
  • s'il n'existe pas on le crée (mkdrip) ;
  • on copie les examples dans /tmp/nodebook-examples (on peut effacer avant) (copy, remove) ;
  • on informe l'utilisateur que la copie s'est bien passée (ou pas).

j'imagine que le code suivant peut-être amélioré, je débute en lisant les exemples du livre 😓

Copy link
Owner

@thom4parisot thom4parisot left a comment

Choose a reason for hiding this comment

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

Bah c'est très bien comme code ! Si tu le comprends et que tu apprends en l'écrivant c'est encore mieux :-)
Je trouve que c'est un excellent moyen d'apprendre.

C'est d'autant plus une bonne idée que l'exemple initial ne rendait pas justice ni l'illustrait vraiment ce que fs-extra permet de faire.

Si je comprends bien tu veux :

  1. t'assurer que le répertoire tmp/node-examples soit créé à la racine du repo s'il n'existe pas ;
  2. copier le contenu du répertoire d'exemples dans le dit-répertoire.

Le seul problème que je vois réside dans le fait que fs.ensureDir et rm sont asynchrones donc en pratique,rm peut se terminer avant fs.ensureDir.

Rien de méchant mais si tu veux respecter un ordre précis va falloir trouver un moyen de garantir que la copie s'exécute après fs.mkdirp.

Tu me dis et on essaiera de trouver une solution ensemble :-)

const fs = require('fs-extra');

fs.copy(__dirname, '/tmp/nodebook-examples', (err) => {
Copy link
Owner

Choose a reason for hiding this comment

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

L'intention initiale c'est de copier le répertoire contenant le script fs-extra.js dans /tmp/nodebook-examples.

const fs = require('fs-extra');

fs.copy(__dirname, '/tmp/nodebook-examples', (err) => {
const dirpath = join(__dirname, '..', '..', 'tmp', 'nodebook-examples');
Copy link
Owner

Choose a reason for hiding this comment

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

Du coup cette ligne change l'emplacement de copie.

Ce qui est un plus je pense car /tmp/nodebook-examples ne fonctionnerait pas sous Windows.

On est bien d'accord que tu veux copier vers la racine du repo ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

oui, j'ai gardé la même logique que le script mkdirp

@@ -1,9 +1,26 @@
'use strict';

const mkdirp = require('mkdirp');
Copy link
Owner

Choose a reason for hiding this comment

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

Vu que tu utilises fs.mkdirp, cette dépendance n'est plus nécessaire.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

oh merci, un bel exemple de cargo culte 😉

}
})

rm(dirpath, () => {
Copy link
Owner

Choose a reason for hiding this comment

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

fs-extra contient une méthode .remove() qui rend l'inclusion de rimraf superflue.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Top, je vais refactoriser ça.

@DirtyF
Copy link
Collaborator Author

DirtyF commented Jan 30, 2017

Pour s'assurer de l'ordre, il faudrait une promesse qui retourne que le dossier a bien été crée et en cas de succès on copie le dossier ?

@thom4parisot
Copy link
Owner

thom4parisot commented Jan 30, 2017

@DirtyF c'est sûrement le plus propre oui – t'as un exemple dans la section Design Pattern : Promesses du chapitre 3, avec pify.

@thom4parisot
Copy link
Owner

thom4parisot commented Feb 1, 2017

@DirtyF je t'invite à jeter un œil à ef5b197 pour voir le résultat final :-)

Merci pour ton aide, j'aurais pas pensé à améliorer cet exemple :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants