-
Notifications
You must be signed in to change notification settings - Fork 52
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
[BUGFIX] Correction du script de configuration #9308
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 : |
@@ -92,10 +92,48 @@ function setup_and_run_infrastructure() { | |||
echo "Creating database" | |||
|
|||
# It drops and creates database then load the seed. | |||
(cd api && npm run db:reset) | |||
(cd api && npm ci && npm run db:reset) |
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.
Ligne 80, je constate qu'on utilise nos docker compose, l'image buildée pour le compose de l'API vient de Dockerfile.hapi
est fait déjà ce npm ci
.
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.
Vu avec @VincentHardouin : les fichiers Docker auxquels il fait référence sont ceux qui sont dans le répertoire docker/
, alors que le script scripts/configure.sh
utilise, lui, le fichier compose.yaml
. Donc les propositions de @theotime2005 sont correctes.
|
||
# Install dependencies for admin | ||
echo "Installing dependencies for admin" | ||
(cd admin && npm ci) |
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.
Pour toutes les apps Ember.js, on utilise l'image faite par Dockerfile.ember
et elle fait déjà l'installation des dépendances
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.
Sauf que dans le guide d'installation, on parle de l'environnement de développement et non de l'environnement de production. Or il est plus simple et logique de faire tourner chaque applications séparément que toutes ensemble et donc d'installer les modules séparément.
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.
Vu avec @VincentHardouin : les fichiers Docker auxquels il fait référence sont ceux qui sont dans le répertoire docker/
, alors que le script scripts/configure.sh
utilise, lui, le fichier compose.yaml
. Donc les propositions de @theotime2005 sont correctes.
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.
Les corrections proposées pour ce script sont correctes.
Comme cela a été évoqué, démarrer toutes ces applications ensemble est très lourd pour un ordinateur et ne correspond pas à l'usage qu'ont les développeurs. Aussi nous gagnerions à mettre à jour la doc et ce script pour proposer quelque chose de plus pratique et qui correspond à l'usage des développeurs. Mais au moins maintenant ce script est fonctionnel.
@@ -92,10 +92,48 @@ function setup_and_run_infrastructure() { | |||
echo "Creating database" | |||
|
|||
# It drops and creates database then load the seed. | |||
(cd api && npm run db:reset) | |||
(cd api && npm ci && npm run db:reset) |
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.
Vu avec @VincentHardouin : les fichiers Docker auxquels il fait référence sont ceux qui sont dans le répertoire docker/
, alors que le script scripts/configure.sh
utilise, lui, le fichier compose.yaml
. Donc les propositions de @theotime2005 sont correctes.
|
||
# Install dependencies for admin | ||
echo "Installing dependencies for admin" | ||
(cd admin && npm ci) |
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.
Vu avec @VincentHardouin : les fichiers Docker auxquels il fait référence sont ceux qui sont dans le répertoire docker/
, alors que le script scripts/configure.sh
utilise, lui, le fichier compose.yaml
. Donc les propositions de @theotime2005 sont correctes.
…'installation des différents node_module dans les dossiers. A garder??
bff9085
to
804ad23
Compare
🦄 Problème
Le script configure.sh ne permet pas d'obtenir un environnement de développement complet.
🤖 Proposition
Ajout des commandes pour installer les dépendances nécessaire à chaque application.
🌈 Remarques
Noter que cette correction ne permet pas de lancer tous les services via une seule commande comme indiqué dans le guide d'installation. Ce point serait aussi à corriger dans une autre PR.
💯 Pour tester
Supprimer tous les dossiers node_modules et exécuter la commande
Vérifier que l'ensemble est fonctionnel et notamment la présence des dossiers node_modules