-
-
Notifications
You must be signed in to change notification settings - Fork 73
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
Conversion du code vers ES2015 #63
Conversation
@oncletom grosse PR 😨 T'as converti avec un outil, t'as linté le code ? Faut tester que tous les codes fonctionnent ? |
J'ai converti à la main ;-) J'ai linté (travis le fait aussi). J'ai testé quelques examples, pas tous. Je dirais qu'il faudrait juste essayer ceux qui paraissent "dangereux". Et je ferai un truc pour tester les examples à plus grande échelle plus tard (juste changer de version de Node ou une dépendance peut péter des trucs) |
T'aurais pu utiliser Lebab. |
@oncletom Souhaites tu effectuer une transformation auto ou pas du coup ? Y'a t-il des règles à suivre ? (utiliser seulement 'use strict';
var cycles = 0;
while(cycles++ < 1000000000);
module.exports = cycles; Avec Lebab : const cycles = 0;
while(cycles++ < 1000000000);
export default cycles; On voit que le mode strict saute, Si tu as transformé à la main, il y a surement une bonne raison et une transformation de syntaxe sera peut-être moins pédagogique dans certains cas. |
Merci @gmarty pour suggérer Y'a pas mal d'éléments de conversion qui sont manquants (d'après la doc) ou unsafe. Ça mange pas de pain de sélectionner des flags de conversion, de voir ce que ça donne dans une PR et de comparer les deux. Dans le cas de ce livre, j'ai envie de garder l'export en module CommonJS pour que le code soit interprétable par Node.js nativement. C'est bon de savoir que le |
Le Je viens de tester en node |
@oncletom En ne sélectionnant que les options safe de Les deux seules exceptions mineures :
--process.stdin.on('data', function(){});
++process.stdin.on('data', () => {});
--table.push.apply(table, modules);
++table.push(...modules); Ça c'est pour la syntaxe, maintenant faut encore tester avec node 6.9.4 |
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.
LGTM 👍 (syntaxement parlant)
@@ -1,19 +1,14 @@ | |||
'use strict'; | |||
|
|||
var Users = require('../lib/models/user.js'); | |||
var argv = require('yargs') | |||
const Users = require('../lib/models/user.js'); |
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.
FYI ce fichier n'est pas présent et génère une erreur lors de l'exécution en ligne de commande :
❯ node chapter-03/examples/src/snippets/cli-app.js
module.js:471
throw err;
^
Error: Cannot find module '../lib/models/user.js'
Merci @DirtyF ! Je pense que j'ai bourdé et soit jamais commité @SebastienElet ah, merci pour le complément ! Ça va me servir pour #4. |
fix #55