WIP: Some little improvements #169
Conversation
| fi | ||
| # Following test is needed otherwise `git clean` remove also untracked files not | ||
| # added into (.gitignore). | ||
| @if [[ "$$(git ls-files --others --exclude-standard | wc -l)" -gt 0 ]] ; \ |
There was a problem hiding this comment.
@jibidus je ne suis pas persuadé que ce soit cool qu'à chaque yarn start on doive être avec aucun untracked file et aucune modification non présente dans la stagging area.
Pour le build, ça me paraît être une bonne pratique.
Mais pour yarn start ça va juste être relou.
Non ?
There was a problem hiding this comment.
Pourquoi on aurait besoin de supprimer des fichiers sur un yarn build ou yarn start d'ailleurs ?
Ca parait bien sournois, je ne pense pas qu'un dev s'attende a voir disparaître des fichiers sur ce genre de commande.
There was a problem hiding this comment.
Après le but c'est justement que y'ait pas de fichiers qui disparaissent sournoisement. C'est le cas à l'heure actuelle, c'est une erreur de ma part (désolé).
| else \ | ||
| echo -e "\nCool, you have no untracked files not included" \ | ||
| "into '.gitignore'\n" ; \ | ||
| fi |
There was a problem hiding this comment.
Cela me paraît bien compliqué tout ça (beaucoup de signes cabalistiques qui rendent la lecture difficile), il n'y a pas moyen de faire plus simple ?
There was a problem hiding this comment.
Je crois que faire un git clean avant un build de mise en prod c'est une bonne pratique. GitLab CI fait comme ça.
Après pour des builds locaux y'a moyen de faire autrement oui.
There was a problem hiding this comment.
Je serais intétéressé de comprendre pourquoi tu trouves cela compliqué ?
C'est du Bash et du Makefile.
There was a problem hiding this comment.
C'est compliqué pour moi parce que juste a la lecture, je ne peux pas voir rapidement s'il y a un problème (s'il manque un ", un \, un $ ou autre par exemple). Et je pense que c'est pareil pour toi qui en est pourtant l'auteur.
There was a problem hiding this comment.
Ah mince, pour moi ça va. D'autant plus que j'ai la coloration syntaxique ;-) .
There was a problem hiding this comment.
Et chez moi les \n et tout ils apparaissent en rouge quand j'écris et tout ;-) . Et les dollards donnent une autre colloration syntaxique.
Après j'ai bien testé.
There was a problem hiding this comment.
Je suis vraiment désolé de faire du code pas clair. Je te propose qu'on se téléphone un jour oû tu auras le temps pour essayer de voir ensemble comment procéder pour fixer le problème.
|
@jibidus conclusion, tu souhaites que je propose une autre solution ? |
JulioJu
force-pushed
the
some-little-improvements
branch
2 times, most recently
from
e86141e
to
32c4da1
Compare
JulioJu
force-pushed
the
some-little-improvements
branch
from
32c4da1
to
8e9a409
Compare
| @@ -12,7 +12,8 @@ start: | |||
| # -v, --verbose verbose output | |||
| yarn hugo server -s site -v | |||
|
|
|||
| start_netlify: | |||
| # Target to test netlify-cms locally | |||
There was a problem hiding this comment.
[next time] le mot Target n'apporte pas d'info (on sait qui est dans un Makefile, donc qu'on est en face d'un taget). Cf Poorly Written Comment.
There was a problem hiding this comment.
Ah ok merci pour l'info !
| # Following is advised by GitLab CI https://docs.gitlab.com/ee/ci/yaml/#git-clean-flags | ||
| # Remove untracked files (files referenced into `.gitignore` include), | ||
| # but we exclude `node_modules`. | ||
| # Following command is advised by GitLab CI, read: |
There was a problem hiding this comment.
[next time] Du coup, cela signifie qu'on sous-entend que clean n'est utilisé que par la CI. Ca ressemble a une dépendance logique du coup (l'implem de clean dépend de où clean est utilisé), ce qu'il faut éviter (cf Make logical dependencies physical).
There was a problem hiding this comment.
Pas forcément non.
Bon je vois qu'il faut que j'améliore cette PR.
| rm -f ./site/static/admin/netlify-cms.js* | ||
| cp ./node_modules/netlify-cms/dist/netlify-cms.js* ./site/static/admin/ | ||
| # Remove Hugo temp files | ||
| rm -rf site/resources/ |
There was a problem hiding this comment.
J'ai l'impression que ce rm fait un clean conceptuellement, non ?
Si oui, alors il devrait être dans le target clean je pense.
PS : idem peut-être pour les autres rm / cp
|
|
||
| _build_common: | ||
| _netlify-build-common: |
There was a problem hiding this comment.
Qui dit netlify-xxx, dit target exécuté par Netlify lors du "déploiement", non ?
Si oui, alors pourquoi faire tous ces checks vu qu'on contrôle complètement ce qui est fait lors du build ?
There was a problem hiding this comment.
Parce que rien n'interdit d'essayer de déployer en local ;-) . Et ça coûte pas cher :-)
| /* As it, the bar is on the baseline */ | ||
| /* But as the font have a height, so strange to be on the baseline */ | ||
| /* On boostrap, we have line-height: 1.5 */ | ||
| /* Therefore if we want that the hyphen has 0.4 em of blanck space at bottom */ |
There was a problem hiding this comment.
| /* Therefore if we want that the hyphen has 0.4 em of blanck space at bottom */ | |
| /* Therefore if we want that the hyphen has 0.4 em of blank space at bottom */ |
There was a problem hiding this comment.
Oupse ! Merci pour ta vigilence.
| /* Therefore if we want that the hyphen has 0.4 em of blanck space at bottom */ | ||
| /* we must have a margin top of 0.9 em */ | ||
| /* We have 1.3 + 0.2 === 1.5 */ | ||
| /* And 1.3 === 0.9 + 0.4 */ | ||
| height: 0.2em; |
There was a problem hiding this comment.
[Out of scope] On pourrait utiliser des variables pour exprimer une bonne partie de ce qui est en commentaire.
Pas sûr que ce soit possible en css, mais je pense que ça l'est en sass par exemple.
There was a problem hiding this comment.
Bien sûr !
Oui y'a des variables en CSS. Je note d'améliorer ce point
No description provided.