-
Notifications
You must be signed in to change notification settings - Fork 28
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
Update openfisca installation process #384
Conversation
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.
Il y a des instructions à modifier / supprimer ll. 126-128.
|
||
```sh | ||
cd mes-aides-ui | ||
pip install -r openfisca/requirements.txt |
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.
Ne peut-on pas mettre ça dans le postinstall
, ou au moins dans un script NPM non lifecycle ?
Manque |
|
||
```sh | ||
cd mes-aides-ui | ||
pip install -r openfisca/requirements.txt |
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.
--user
(ou documenter virtualenv)
Je fais |
GTM. J'ajouterais bien des scripts NPM. |
2e9fd5a
to
790a540
Compare
790a540
to
3810d07
Compare
Done. |
"db-update": "./import-tests.sh" | ||
"db-update": "./import-tests.sh", | ||
"install-openfisca": "pip install --user --upgrade -r openfisca/requirements.txt", | ||
"openfisca":"paster serve openfisca/api_config.ini" |
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.
Manque un espace après les deux points ;)
@@ -72,7 +72,9 @@ | |||
"test-integration": "test/integration/run-integration-tests.sh", | |||
"predb": "mkdir -p db", | |||
"db": "mongod --dbpath db", | |||
"db-update": "./import-tests.sh" | |||
"db-update": "./import-tests.sh", | |||
"install-openfisca": "pip install --user --upgrade -r openfisca/requirements.txt", |
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.
Je mettrais plutôt ça dans preopenfisca
, pour que lancer openfisca
via NPM soit toujours à jour de l'état courant de l'application.
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'y vois deux inconvénients :
- Si j'utilise un
virtualenv
, je ne peux pas utiliserinstall-openfisca
. Lier les deux scripts m'empêcherait aussi d'utilisernpm run openfisca
. - Si j'ai installé openfisca en mode éditable pour travailler sur une formule, exécuter la commande
install-openfisca
me réinstallera openfisca à partir du dépôt distant. Lier les deux scripts m'empêche aussi dans ce cas d'utilisernpm run openfisca
.
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 pourrait le mettre ce script en postinstall
, mais ça pose la question suivante: "Est-ce que parfois on veut installer mes-aides sans installer openfisca ?"
|
||
```sh | ||
cd mes-aides-ui | ||
npm run install-openfisca # ou pip install --upgrade -r openfisca/requirements.txt si vous utilisez un environnement virtuel |
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.
Il faut probablement expliciter la dépendance à pip
.
GTM |
Bien vu ! Tant pis :)
|
Non, pour les raisons de modalités d’installation alternatives que tu mentionnais dans un autre commentaire.
Par ailleurs, ça veut dire que l’installation de Mes Aides échouera si on n’a pas `pip` installé. Ça me semble une dépendance forte et pas très claire.
|
No description provided.