Skip to content
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

Build auto du front avec Travis #2482

Merged
merged 8 commits into from
Apr 23, 2015

Conversation

sandhose
Copy link
Contributor

@sandhose sandhose commented Apr 2, 2015

Q R
Correction de bugs ? non
Nouvelle Fonctionnalité ? oui
Tickets (issues) concernés forum, #2471

Permet de commit + push le front buildé via travis dès qu'il y a un push vers une branche type release-XX, ou dès qu'il y a un commit avec dans le message de commit [build front] (n'importe quelle branche)

Les commits sont fait avec l'utilisateur @ZDS-Bot

@coveralls
Copy link

Coverage Status

Coverage remained the same at 79.3% when pulling c567a32 on sandhose:travis-front-build into 4194574 on zestedesavoir:dev.

@pierre-24
Copy link
Member

Donc la question qui tue, maintenant ... Comment est ce qu'on QA ce truc ? :o

@landscape-bot
Copy link

Code Health
Code quality remained the same when pulling c567a32 on sandhose:travis-front-build into 4194574 on zestedesavoir:dev.

@sandhose
Copy link
Contributor Author

sandhose commented Apr 3, 2015

Ca risque effectivement d'être un peu tricky à QA :-°

Donc en gros, ce qu'il faut faire:

  • Activer Travis sur son fork
  • Push cette PR sur son fork
  • Encoder le GITHUB_TOKEN dans le .travis.yml pour que la variable d'environnement soit active. La façon la plus simple de le faire est d'entrer cette variable dans les settings travis. Ce token est un token OAuth que je peux fournir à qui veut. Sinon, vous pouvez utiliser un de vos token oauth, que vous pouvez générer dans les settings GitHub. Pour vérifier que le token est bien injecté, il faut qu'au début des logs travis il y ait $ export GITHUB_TOKEN=[secure]
  • Tester en poussant la branche vers une branche avec un nom type release-XX
  • Tester avec un commit contenant dans le message [build front]

@sandhose
Copy link
Contributor Author

sandhose commented Apr 3, 2015

Je suis en train de me dire que la je check que le dernier message de commit. Est-ce que c'est ce qu'on veut ? Ou il faut check dans les message de toutes les commits qui ont été push avec la build ? (via la variable d'environnement TRAVIS_COMMIT_RANGE)

@firm1
Copy link
Contributor

firm1 commented Apr 3, 2015

Je suis en train de me dire que la je check que le dernier message de commit. Est-ce que c'est ce qu'on veut ? Ou il faut check dans les message de toutes les commits qui ont été push avec la build ? (via la variable d'environnement TRAVIS_COMMIT_RANGE)

Je dirais qu'il faut vérifier dans tous les commits de la PR.

@pierre-24
Copy link
Member

J'ai mal à la tête rien qu'à voir les instructions de QA. Bon, j'y pense, j'y pense, mais j'ai besoin de motivation ^^ (remarque, activer Travis sur mon fork me semble une bonne idée, donc ça peut être l'occaz).

@SpaceFox
Copy link
Contributor

Il va falloir rebase ici :(

@landscape-bot
Copy link

Code Health
Code quality remained the same when pulling d8d0d86 on sandhose:travis-front-build into bba7dd4 on zestedesavoir:dev.

@sandhose
Copy link
Contributor Author

C'est fait ! Par contre le fail sur les tests ne vient pas de cette PR, et est présent sur d'autres build...

@pierre-24 pierre-24 mentioned this pull request Apr 14, 2015
@pierre-24
Copy link
Member

Ok, je bloque sur

Encoder le GITHUB_TOKEN dans le .travis.yml pour que la variable d'environnement soit active. La façon la plus simple de le faire est d'entrer cette variable dans les settings travis. Ce token est un token OAuth que je peux fournir à qui veut. Sinon, vous pouvez utiliser un de vos token oauth, que vous pouvez générer dans les settings GitHub. Pour vérifier que le token est bien injecté, il faut qu'au début des logs travis il y ait $ export GITHUB_TOKEN=[secure]

Deux questions:

  1. Quelles autorisations je dois donner à Travis via mon token ?
  2. Comment par après je peut encoder la variable et la donner à Travis ?

D'avance merci :)

@sandhose
Copy link
Contributor Author

Je crois qu'il faut les autorisations de lecture/ecriture des répo... Pour encoder la variable, soit faut le faire dans le travis.yml (ce qui est déjà fait dans cette PR pour ce répo) via la méthode décrite sur la doc, soit directement dans les settings du répo sur travis ; pas besoin d'encoder ni rien, juste à copy-paste le token généré :o)

@pierre-24
Copy link
Member

Rapport de QA:

Voilà les faits, vous me dites si ça correspond à ce que vous voulez

MAIS !!

Cette fonction possède un side-effect qui a mon avis ne va pas vous plaire du tout: elle rajoute ce qui se passe dans /dist/ dans la liste des choses considérées par Git dans sa liste de fichier, alors que ce n'est pas le cas par défaut à cause du .gitignore. D'une part, ça génère le besoin de faire des merge/rebase le cas échéant, puisque sinon, il y a un commit de plus dans la branche qu'en local. Mais aussi et surtout, ça veux dire que dans la suite, si l'auteur de la branche fait npm run gulp -- build, Git rajoute le build du front au commit, ce qui signifie que c'est le front de l'auteur qui part sur le dépot, et plus celui généré par Travis (avec tout les problèmes que ça peut entrainer). Pire que ça, lorsqu'on merge la branche, ce comportement ne disparait pas. Ce qui signifie que dès qu'on aura fait ça une fois, on peut avoir ce problème en permanence, et ça touchera alors TOUT les développeurs.

En l'état, je peux donc très clairement annoncer que ceci n'est absolument pas une bonne idée et que je peux donc pas cautionner ce qui se passe. Et la solution "on limite ça à la branche de release" est pas une bonne idée, puisque la suite logique d'une release, c'est de remonter les bugfix dans dev. Autrement dit, on sera "contaminé" à moyen terme.

Vraiment désolé, @SpaceFox et @sandhose ... Mais il va à mon avis falloir trouver autre chose.

Et là, je me dis que j'ai fais tout ce bazar pour rien :'(

@sandhose
Copy link
Contributor Author

J'avais effectivement pas pensé a ça... Je pensais que le gitignore évitait
de racket ces fichier... Je solution serait de demander a tout le monde
de faire un "git update-index --assume-unchanged dist/", ce qui est un peu
compliqué...

L'autre solution serait de push un dossier a part (donc déplacer le dist
vers ce dossier lors des builds travis), puis copier le contenu de ce
dossier vers dist/ dans le script de déploiement

Autre chose qui m'a été proposé par Alex récemment — je sais absolument pas
si c'est possible — c'est de build lorsqu'on met a jour un tag, et que l'on
pousse uniquement sur le tag. Le tag sera alors détaché de la branche
release, et lorsque l'on mettra a jour le tag, le commit avec le build
front. Du coup, il suffira de deployer le tag en question sur le serveur
pour avoir les fichiers front a jour. Au passage, ca évite d'alourdir le
repo, puisqu'un dev lambda ne pullera jamais ces fichiers. J'essaierai ce
soir si j'ai le temps :)

Le jeu. 16 avr. 2015 04:04, Pierre Beaujean notifications@github.com a
écrit :

Rapport de QA:

Voilà les faits, vous me dites si ça correspond à ce que vous voulez

  • Demander le push d'une branche quelconque ne génère pas de build
    Pas de build de front pierre-24/zds-site#7: OK
  • Demander le push d'une branche quelconque dans la branche de release
    ne génère pas de build Normalement pas de build ici non plus pierre-24/zds-site#8:
    OK.
  • Demander le push d'une branche possédant un nom de type release-XX
    génère un build Un improbable changement de couleur pierre-24/zds-site#5: OK.
    Par contre, comme vous pouvez le constater, un commit avec [front
    build] fait par après ne fonctionne plus, donc là je dis pas OK.
  • Faire des [build front] sur une PR quelconque fonctionne
    Une PR à la rien à voir pierre-24/zds-site#6. À noter qu'on peut en
    demander plusieurs sur la même PR, et que le build ne se fait pas si le
    message contenant [build front] n'est pas le dernier lors du push. À
    fortiori ok, ok et ok.
  • Ça marche même si Travis plante un test, et ça c'est plutôt bien par
    les temps qui cours.
  • Détail qui n'ennuie que moi, la commande est [build front] et le
    commit dit front build. Comme je dis, ça n'ennuie que moi ^^
  • Je n'accepte pas que cette PR sois mergée sans un bout de doc
    documentant cette fonction, désolé.

MAIS !!

Cette fonction possède un side-effect qui a mon avis ne va pas vous
plaire du tout: elle rajoute ce qui se passe dans /dist/ dans la liste
des choses considérées par Git dans sa liste de fichier, alors que ce n'est
pas le cas par défaut à cause du .gitignore. D'une part, ça génère le
besoin de faire des merge/rebase le cas échéant, puisque sinon, il y a un
commit de plus dans la branche qu'en local. Mais aussi et surtout, ça veux
dire que dans la suite, si l'auteur de la branche fait npm run gulp --
build, Git rajoute le build du front au commit, ce qui signifie que c'est
le front de l'auteur qui part sur le dépot, et plus celui généré par Travis
(avec tout les problèmes que ça peut entrainer). Pire que ça, lorsqu'on
merge la branche, ce comportement ne disparait pas. Ce qui signifie que dès
qu'on aura fait ça une fois, on peut avoir ce problème en permanence, et ça
touchera alors TOUT les développeurs.

En l'état, je peux donc très clairement annoncer que ceci n'est absolument
pas une bonne idée et que je peux donc pas cautionner ce qui se passe. Et
la solution "on limite ça à la branche de release" est pas une bonne idée,
puisque la suite logique d'une release, c'est de remonter les bugfix dans
dev. Autrement dit, on sera "contaminé" à moyen terme.

Vraiment désolé, @SpaceFox https://github.com/SpaceFox et @sandhose
https://github.com/sandhose ... Mais il va falloir trouver autre chose.


Reply to this email directly or view it on GitHub
#2482 (comment)
.

@sandhose
Copy link
Contributor Author

D'après la doc git, créer un tag détaché d'une branche serait possible (par
contre j'ai pas pu tester, je suis dans le train sur mon portable...). La
seule contrainte serait de créer a chaque fois un nouveau tag, puisque
modifier un tag existant est très déconseillé... Du coup, on pourrait
pousser vers v1.7-build, par exemple, et c'est ce tag qui sera utilisé lors
du déploiement (les tags de build pourront évidement être virés plus tard).
Il me faudrait ton avis, @SpaceFox, puisque c'est toi le premier concerné
par ces histoires de déploiement :)

Le jeu. 16 avr. 2015 07:32, Quentin Gliech quentingliech@gmail.com a
écrit :

J'avais effectivement pas pensé a ça... Je pensais que le gitignore
évitait de racket ces fichier... Je solution serait de demander a tout le
monde
de faire un "git update-index --assume-unchanged dist/", ce qui est
un peu compliqué...

L'autre solution serait de push un dossier a part (donc déplacer le dist
vers ce dossier lors des builds travis), puis copier le contenu de ce
dossier vers dist/ dans le script de déploiement

Autre chose qui m'a été proposé par Alex récemment — je sais absolument
pas si c'est possible — c'est de build lorsqu'on met a jour un tag, et que
l'on pousse uniquement sur le tag. Le tag sera alors détaché de la
branche release, et lorsque l'on mettra a jour le tag, le commit avec le
build front. Du coup, il suffira de deployer le tag en question sur le
serveur pour avoir les fichiers front a jour. Au passage, ca évite
d'alourdir le repo, puisqu'un dev lambda ne pullera jamais ces fichiers.
J'essaierai ce soir si j'ai le temps :)

Le jeu. 16 avr. 2015 04:04, Pierre Beaujean notifications@github.com a
écrit :

Rapport de QA:

Voilà les faits, vous me dites si ça correspond à ce que vous voulez

  • Demander le push d'une branche quelconque ne génère pas de build
    Pas de build de front pierre-24/zds-site#7: OK
  • Demander le push d'une branche quelconque dans la branche de
    release ne génère pas de build
    Normalement pas de build ici non plus pierre-24/zds-site#8: OK.
  • Demander le push d'une branche possédant un nom de type release-XX
    génère un build Un improbable changement de couleur pierre-24/zds-site#5: OK.
    Par contre, comme vous pouvez le constater, un commit avec [front
    build] fait par après ne fonctionne plus, donc là je dis pas OK.
  • Faire des [build front] sur une PR quelconque fonctionne
    Une PR à la rien à voir pierre-24/zds-site#6. À noter qu'on peut en
    demander plusieurs sur la même PR, et que le build ne se fait pas si le
    message contenant [build front] n'est pas le dernier lors du push. À
    fortiori ok, ok et ok.
  • Ça marche même si Travis plante un test, et ça c'est plutôt bien
    par les temps qui cours.
  • Détail qui n'ennuie que moi, la commande est [build front] et le
    commit dit front build. Comme je dis, ça n'ennuie que moi ^^
  • Je n'accepte pas que cette PR sois mergée sans un bout de doc
    documentant cette fonction, désolé.

MAIS !!

Cette fonction possède un side-effect qui a mon avis ne va pas vous
plaire du tout: elle rajoute ce qui se passe dans /dist/ dans la liste
des choses considérées par Git dans sa liste de fichier, alors que ce n'est
pas le cas par défaut à cause du .gitignore. D'une part, ça génère le
besoin de faire des merge/rebase le cas échéant, puisque sinon, il y a un
commit de plus dans la branche qu'en local. Mais aussi et surtout, ça veux
dire que dans la suite, si l'auteur de la branche fait npm run gulp --
build, Git rajoute le build du front au commit, ce qui signifie que
c'est le front de l'auteur qui part sur le dépot, et plus celui généré par
Travis (avec tout les problèmes que ça peut entrainer). Pire que ça,
lorsqu'on merge la branche, ce comportement ne disparait pas. Ce qui
signifie que dès qu'on aura fait ça une fois, on peut avoir ce problème en
permanence, et ça touchera alors TOUT les développeurs.

En l'état, je peux donc très clairement annoncer que ceci n'est
absolument pas une bonne idée et que je peux donc pas cautionner ce qui se
passe. Et la solution "on limite ça à la branche de release" est pas une
bonne idée, puisque la suite logique d'une release, c'est de remonter les
bugfix dans dev. Autrement dit, on sera "contaminé" à moyen terme.

Vraiment désolé, @SpaceFox https://github.com/SpaceFox et @sandhose
https://github.com/sandhose ... Mais il va falloir trouver autre chose.


Reply to this email directly or view it on GitHub
#2482 (comment)
.

@landscape-bot
Copy link

Code Health
Code quality remained the same when pulling b679e1b on sandhose:travis-front-build into bba7dd4 on zestedesavoir:dev.

@sandhose
Copy link
Contributor Author

Bon, c'est effectivement possible de push uniquement sur un tag !

Du coup, le nouveau comportement, c'est que dès qu'un tag est créé (ce qui arrive forcément à chaque release), il va commit en ajoutant les fichiers front générés, et créer un nouveau tag [tag]-build (genre si le tag de base s'appelle v1.8, le nouveau tag s'appellera v1.8-build).

Du coup, lors des MEP, il suffit de checkout le tag *-build (donc lancer le deploy.sh avec le bon tag), et le tour est joué ! Puisque ces commits ne se trouveront sur aucune branche, on aura pas d'historique hyper-lourd, et pas de conflits sur ces fichiers :)

Du coup, je vais juste encore modifier le deploy.sh pour virer toute la partie build du front (et éventuellement rajouter un rm -rf dist/ pour être sur que git râle pas lors du checkout du tag), et écrire le petit bout de doc qu'il faut à ce sujet

Instructions QA

Du coup, pour le nouveau comportement, il faut activer travis, et rajouter la variable d'environnement comme expliqué plus haut, vérifier que ça fait rien quand il y a pas de tag, et vérifier que ça créé un nouveau tag avec le bon nom lorsqu'on push un tag (en gros git tag -a nom-du-tag et git push origin nom-du-tag). La liste des tags se trouve dans le menu déroulant des branches sur GitHub

@pierre-24
Copy link
Member

Je vais tenter de faire ça ASAP. Par contre, le fait que Travis nous plante est lié à cette PR ou c'est juste NPM qui a décidé de nous rappeler son existence ?

D'ailleurs, ça m'amène à une question générale: qu'est ce qui va se passer si NPM se loupe pour une raison ou une autre ? Est ce qu'il y a moyen de relancer le build ?

@sandhose
Copy link
Contributor Author

Le fail travis viens d'un timeout lorsqu'il a tenté de récupérer les binaries de lwip (module de manipulation d'image qui sert dans la génération du sprite).

De manière générale, si le build fail, suffit de le relancer ; il poussera rien si le build fail (le script est dans after_success)

@pierre-24
Copy link
Member

Cool. Si ça marche, ça va être énorme :)

@sandhose
Copy link
Contributor Author

Question: est-ce que sur la préprod c'est des tags ou des branches qui sont déployées ?

Parce que si c'est pas des tags, bah le front sera pas build... Du coup, soit faudra changer le deploy.sh pour rajouter un argument s'il faut build le front ou non, soit faudra build "à la main" à chaque MEP sur la préprod (ce qui se résume par npm install && npm run build...)

@Situphen Situphen added the C-Front Concerne l'interface du site label Apr 21, 2015
@Situphen
Copy link
Member

Oo
Ça a l'air super cool !

Il reste à modifier le script de déploiement c'est ça ?

@pierre-24
Copy link
Member

Et de la doc :) (mais je sais que @Situphen adooore relire la doc)

@landscape-bot
Copy link

Code Health
Code quality remained the same when pulling 45152ea on sandhose:travis-front-build into bba7dd4 on zestedesavoir:dev.

@sandhose
Copy link
Contributor Author

Suite aux remarques de @Taluu, j'ai déplacé toute la partie "push du front" du travis.yml vers scripts/push_front.sh. J'en ai du coup profité pour déplacer le deploy.sh vers ce nouveau dossier scripts/. @SpaceFox: Ca te convient ?

J'ai aussi écrit le petit bout de doc qu'il faut ; j'attends juste le résultat d'un build dû à un push de tag pour vérifier que le script marche, et sinon c'est bon pour merge :)

@@ -12,6 +12,8 @@ Il s'agit donc de la partie du code définissant le design et l'affichage, mais

`NodeJS (en) <https://nodejs.org/>`__, `NPM (en) <https://www.npmjs.com/>`__ (gestionaire de paquet pour NodeJS) et `Gulp (en) <http://gulpjs.com/>`__ sont utilisés pour générer le code final minifié et cohérent. Le développement du *front-end* requiert donc des outils spécifiques dont l'installation `est expliquée ici <install/frontend-install.html>`__.
Copy link
Member

Choose a reason for hiding this comment

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

Tu peux corriger "gestionaire" vu que c'est pas loin de ton bout de texte ? :)

@landscape-bot
Copy link

Code Health
Code quality remained the same when pulling d5f3bf2 on sandhose:travis-front-build into bba7dd4 on zestedesavoir:dev.

`NodeJS (en) <https://nodejs.org/>`__, `NPM (en) <https://www.npmjs.com/>`__ (gestionaire de paquet pour NodeJS) et `Gulp (en) <http://gulpjs.com/>`__ sont utilisés pour générer le code final minifié et cohérent. Le développement du *front-end* requiert donc des outils spécifiques dont l'installation `est expliquée ici <install/frontend-install.html>`__.
`NodeJS (en) <https://nodejs.org/>`__, `NPM (en) <https://www.npmjs.com/>`__ (gestionnaire de paquet pour NodeJS) et `Gulp (en) <http://gulpjs.com/>`__ sont utilisés pour générer le code final minifié et cohérent. Le développement du *front-end* requiert donc des outils spécifiques dont l'installation `est expliquée ici <install/frontend-install.html>`__.

Pour éviter d'installer les outils front en production pour des questions de fiabilité, le front est automatiquement compilé par travis et poussé sur le dépot dès qu'un tag (qui correspond à une release) est poussé sur GitHub. `scripts/push_front.sh <https://github.com/zestedesavoir/zds-site/tree/dev/scripts/push_front.sh>`__ est donc lancé avec l'utilisateur `ZDS-Bot <https://github.com/zds-bot>`__ dès qu'un tag est poussé sur le dépot. Ce script crée un nouveau tag avec *-build* en suffixe, contenant un commit avec le front compilé, qui sera déployé en (pré-)production.
Copy link
Member

Choose a reason for hiding this comment

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

"compilé" --> "généré" ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Les deux vont ? Le SCSS est compilé, le JS l'est aussi plus ou moins... Boarf, tu me dira, c'est vite changé !

@Situphen
Copy link
Member

Les droits du fichier deploy.sh ont changé (100644 → 100755) : il faudrait les remettre comme ils étaient.

@landscape-bot
Copy link

Code Health
Repository health decreased by 1% when pulling 619d2f0 on sandhose:travis-front-build into b139a5a on zestedesavoir:doc-tags.

@sandhose
Copy link
Contributor Author

Les droits u fichier deploy.sh ont changé (100644 → 100755) : il faudrait les remettre comme ils étaient.

En fait ça m'a paru bizarre qu'il soit pas exécutable, et comme j'avais rendu de toutes façons le push_front.sh exécutable, j'ai fait de même pour le deploy.sh. Y'a une raison à ce qu'il soit en 644 ?

@sandhose
Copy link
Contributor Author

J'ai rétabli les permissions en 644 sur deploy.sh ; par contre, travis refuse de build depuis un petit moment :x https://travis-ci.org/sandhose/zds-site/builds

@pierre-24
Copy link
Member

Y'a apt-get qui nous plante une dépendance :/

@landscape-bot
Copy link

Code Health
Repository health decreased by 1% when pulling 4fe4b94 on sandhose:travis-front-build into b139a5a on zestedesavoir:doc-tags.

1 similar comment
@landscape-bot
Copy link

Code Health
Repository health decreased by 1% when pulling 4fe4b94 on sandhose:travis-front-build into b139a5a on zestedesavoir:doc-tags.

@sandhose
Copy link
Contributor Author

@pierre-24 Nan mais la c'est vraiment qu'il lance pas le build ! :(

https://travis-ci.org/sandhose/zds-site/jobs/59427033

@pierre-24
Copy link
Member

Travis a des erreurs sous Linux: http://www.traviscistatus.com/ (puis je crois qu'ils sont en maintenance de ce coté là)

@coveralls
Copy link

Coverage Status

Coverage remained the same at 79.58% when pulling 4fe4b94 on sandhose:travis-front-build into b139a5a on zestedesavoir:dev.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 79.58% when pulling 4fe4b94 on sandhose:travis-front-build into b139a5a on zestedesavoir:dev.

@sandhose
Copy link
Contributor Author

Etant donné que le build a passé avec les modifs récentes, cette PR est prête à être merge.

@pierre-24
Copy link
Member

Je ne vois rien qui s'y oppose :)

pierre-24 added a commit that referenced this pull request Apr 23, 2015
@pierre-24 pierre-24 merged commit 751f8a6 into zestedesavoir:dev Apr 23, 2015
@sandhose sandhose deleted the travis-front-build branch April 23, 2015 18:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Front Concerne l'interface du site
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants