Skip to content

Conversation

@christopheNan
Copy link
Contributor

Peut aider à ne pas se retrouver avec #1857

Copy link
Collaborator

@jeanas jeanas left a comment

Choose a reason for hiding this comment

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

Oui, c'est beaucoup mieux !

@christopheNan
Copy link
Contributor Author

Should help for #1863.

jeanas
jeanas previously approved these changes May 7, 2022
Copy link
Collaborator

@jeanas jeanas left a comment

Choose a reason for hiding this comment

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

Merge quand tu veux.

@PyDocTeur
Copy link

C'est bien parti ! Ça manque d'un label automerge ?


Disclaimer

Je suis un robot fait par l'équipe de l'AFPy et de Traduction
sur leur temps libre. Je risque de dire des bétises. Ne me blâmez pas, blamez les développeurs.

Code source

I'm a bot made by the Translation and AFPy teams on their free
time. I might say or do dumb things sometimes. Don't blame me, blame the developer !

Source code

(state: approved)
PyDocTeur v1.12.0

- Utilisation de la variable PYTHON plutôt que VENVDIR (afin de n'avoir à
redéfinir que PYTHON)
- extension aux règles serve, todo, wrap, spell
@JulienPalard
Copy link
Member

update_venv dépend de ensure_prerequisites, ça ne peut pas fonctionner :

$ make update_venv
Déjà sur '3.10'
Votre branche est à jour avec 'origin/3.10'.
You're missing dependencies please install using:

make update_venv
make: *** [Makefile:115 : ensure_prerequisites] Erreur 1

Makefile Outdated
mkdir -p locales/$(LANGUAGE)/LC_MESSAGES/
$(CP_CMD) -u --parents *.po */*.po locales/$(LANGUAGE)/LC_MESSAGES/
$(MAKE) -C venv/cpython/Doc/ \
PYTHON=../../../$(PYTHON) \
Copy link
Member

Choose a reason for hiding this comment

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

J'ignore si $(abspath $(PYTHON)) ne serait pas mieux? Pas sur.

@JulienPalard
Copy link
Member

Pour moi cette PR ajoute trop de complexité, et je doute qu'elle résolve le problème initial. Enfin elle simplifie sa résolution, l'utilisateur devra taper make update-venv plutôt qu'aller glisser pip install --upgrade dans le bon environnement ?

Mais je ne pense pas (ou je ne pense plus) que le rôle du Makefile soit de s'occuper de tout et de tout contrôler pour tout faire fonctionner dans tous les cas.

D'ailleurs le Makefile actuel n'impose pas grand chose à part d'avoir les dépendances d'installées. Il n'impose pas d'utiliser un venv (ni côté trad ni côté cpython), ce que cette PR changerait.

Après tout, pour moi, si un nouveau veut juste faire un pip install sans venv (qui va donc installer dans son ~/.local, rien de méchant), ça me va. S'il veut utiliser pipx ou n'importe quoi d'autre, même son gestionnaire de paquets, tant qu'il arrive à tirer les dépendances, moi ça me va.

Pour vraiment résoudre le problème des personnes qui ont des dépendances pas à jour il faudrait faire tourner un pip install à chaque invocation de make, ce qui nécessite du réseau, est lent, est n'est probablement juste pas le travail de make.

@christopheNan
Copy link
Contributor Author

Pour moi cette PR ajoute trop de complexité, et je doute qu'elle résolve le problème initial. Enfin elle simplifie sa résolution, l'utilisateur devra taper make update-venv plutôt qu'aller glisser pip install --upgrade dans le bon environnement ?

Ok

Mais je ne pense pas (ou je ne pense plus) que le rôle du Makefile soit de s'occuper de tout et de tout contrôler pour tout faire fonctionner dans tous les cas.

D'ailleurs le Makefile actuel n'impose pas grand chose à part d'avoir les dépendances d'installées. Il n'impose pas d'utiliser un venv (ni côté trad ni côté cpython), ce que cette PR changerait.

Celui-ci non plus. Make VENVDIR=/usr/ doit fonctionner (pas testé cependant).

Après tout, pour moi, si un nouveau veut juste faire un pip install sans venv (qui va donc installer dans son ~/.local, rien de méchant), ça me va. S'il veut utiliser pipx ou n'importe quoi d'autre, même son gestionnaire de paquets, tant qu'il arrive à tirer les dépendances, moi ça me va.

Ok

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants