-
Notifications
You must be signed in to change notification settings - Fork 57
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
[DOCS] S'assurer d'une version minimale de Node.js #6513
Conversation
Une fois les applications déployées, elles seront accessibles via les liens suivants : Les variables d'environnement seront accessibles via les liens suivants : |
2004809
to
0d5e1d5
Compare
0d5e1d5
to
c16f2f8
Compare
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.
👍
- Alerte en local que la version utilisée n'est pas bonne. | ||
|
||
#### Inconvénients | ||
- Nécessite des `nvm install` plus réguliers. |
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.
Pas certain que tout le monde utilise nvm. L'inconvéniant c'est de devoir installer une nouvelle version de node plus régulièrement ainsi que la reinstallation des dépendances.
Cela dit je pense que c'est compréhensible par tou.te.s
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.
Si on lance le npm ci ou le npm install (les commandes que je lance tous les jours dans ma routine de dev à la différence de nvm install
) le hook de preinstall (appelé par ci et install, voir la doc qui fait un check-engine va remonter une erreur et je sais que dois executer nvm install (ou autre méthode pour mettre à jour ma version de node)
- Nécessite des `nvm install` plus réguliers. |
Il faut de toute manière mettre à jour la version de node locale en cas de bug node dans les 3 solutions.
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.
J'étais persuadé qu'on avait préconisé l'usage de nvm dans le guide d'installation mais ce n'est pas le cas. Il faudrait qu'on mette à jour ce doc pour préciser où trouver les versions plutôt que les mettre en dur dans la doc ^^"
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.
On en parle plusieurs fois dans les ADR par contre.
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.
@annemarie35 effectivement dans les 3 solutions on est plus stricts qu'avant donc il y aura des remises à jour plus régulières en local. Mais la 3ème solution reste plus flexible que les 2 autres puisqu'on laisse libre la version patch.
On a précisé cette remise à jour nécessaire en dev en inconvénient sur les 3 solutions, par rapport à ce qu'on fait aujourd'hui. On avait précisé dans l'ADR 18 l'usage de check-engine
, est-ce que tu penses qu'il y a d'autres choses à préciser ?
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.
J'étais persuadé qu'on avait préconisé l'usage de nvm dans le guide d'installation mais ce n'est pas le cas. Il faudrait qu'on mette à jour ce doc pour préciser où trouver les versions plutôt que les mettre en dur dans la doc ^^"
J'utilise depuis peu fnm
qui fait la meme chose en plus rapide et fait directement un fnm use
au changement de dossier afin d'utiliser la version spécifié dans le .nvmrc
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.
Pair-review avec @annemarie35 : c'est tout bon.
Merci du travail 🚀
On suggéré des formulation plus claires à notre goût.
4cf0bbe
to
7d7c73d
Compare
Petite question peut être un peu bête mais on a jamais envisagé de faire tourner l'api dans Docker en local pour que tout le monde ait la même version de node (de manière transparente puisque ça serait dans le Dockerfile) ? |
A tester mais il y a quelques années, les performances sur macos étaient catastrophiques, notamment du au |
Il y a déjà ça https://github.com/1024pix/pix/tree/dev/docker |
Je crois que certains Captains utilisent ça effectivement. Pour ma part j'utilise asdf, et on a aussi un fichier de config Nix. Pour le moment on a gardé la liberté sur l'outillage, ça me plait bien de ne rien imposer :) mais à chacun de maintenir ce dont il a besoin. On propose quand même de mettre à jour le |
7d7c73d
to
572fc4d
Compare
Tout tourne sur docker chez moi. |
Je me demande si il y a pas une balance à avoir entre automatiser pour tout le monde (ce que je vois comme une amélioration de la DX) cette partie qui semble poser pas mal de soucis et la liberté de choisir ses outils 🤔 En fait ce que je comprends pas c'est en quoi ça bride le choix des outils que le script |
Pour mieux illustrer les impacts de cet ADR, cette PR montre les changements à venir si il n'y a pas de blocage d'ici demain. |
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 🌸 🌻 🌷 🥀 💮 🌺 🌹
Co-authored-by: Anne-Marie <[email protected]>
Co-authored-by: Anne-Marie <[email protected]>
Co-authored-by: Anne-Marie <[email protected]>
Co-authored-by: Anne-Marie <[email protected]>
Co-authored-by: Anne-Marie <[email protected]>
50e8e94
to
a9a6f69
Compare
🦄 Problème
Depuis l'ADR 18 nous avons quelques limites sur notre manière de préciser la version de Node.js utilisée.
🤖 Proposition
Avec l'équipe Tech Days "UP", nous proposons une remise à jour de cet ADR.
Deadline pour les retours : 4 août 2023.
🌈 Remarques
Ce nouvel ADR fait suite à de nombreuses discussions dans l'équipe, sa conclusion nous parait être le meilleur compromis qu'on puisse proposer pour le moment même si elle induit un petit changement sur notre manière de travailler.
💯 Pour tester
N/A