-
Notifications
You must be signed in to change notification settings - Fork 1
WIP: Some little improvements #169
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| .DEFAULT_GOAL := start | ||
|
|
||
| .PHONY: start start_netlify build build_preview clean _common _build_common | ||
| .PHONY: start start-netlify netlify-build netlify-build-preview clean _common _netlify-build-common | ||
|
|
||
| # ============================================================================== | ||
| # Run Netlify in watch mode | ||
|
|
@@ -12,7 +12,8 @@ start: | |
| # -v, --verbose verbose output | ||
| yarn hugo server -s site -v | ||
|
|
||
| start_netlify: | ||
| # Target to test netlify-cms locally | ||
| start-netlify: | ||
| make _common | ||
| netlify-cms-proxy-server & | ||
| sleep 3 | ||
|
|
@@ -23,16 +24,16 @@ start_netlify: | |
| # ============================================================================== | ||
|
|
||
| # Target used by Netlify to build the prod version. | ||
| build: | ||
| make _build_common | ||
| netlify-build: | ||
| make _netlify-build-common | ||
| # -s, --source string filesystem path to read files relative from | ||
| # -d, --destination string filesystem path to write files to | ||
| # -v, --verbose verbose output | ||
| yarn hugo -s site -d ../dist -v | ||
|
|
||
| # Target used by Netlify to build his preview | ||
| build_preview: | ||
| make _build_common | ||
| netlify-build-preview: | ||
| make _netlify-build-common | ||
| # -s, --source string filesystem path to read files relative from | ||
| # -d, --destination string filesystem path to write files to | ||
| # -D, --buildDrafts include content marked as draft | ||
|
|
@@ -41,28 +42,64 @@ build_preview: | |
| yarn hugo -s site -d ../dist -D -F -v | ||
|
|
||
| clean: | ||
| # We should clean the repo, because Netlify does not clean the repo | ||
| # Options `-f -f` is not an error. See https://github.com/sogilis/Blog/pull/160#discussion_r422919097 | ||
| # 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: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [next time] Du coup, cela signifie qu'on sous-entend que
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pas forcément non. Bon je vois qu'il faut que j'améliore cette PR. |
||
| # https://docs.gitlab.com/ee/ci/yaml/#git-clean-flags | ||
| git clean -f -f -d -x -e node_modules | ||
|
|
||
|
|
||
| # ============================================================================== | ||
| # Private target (used by other targets above, but no sens to use outside) | ||
| # ============================================================================== | ||
|
|
||
| _common: | ||
| make clean | ||
| yarn install --network-timeout 3000 --prefer-offline | ||
| yarn lint | ||
| yarn prettier:check | ||
| # reboot.css | ||
| rm -f ./site/themes/sogilis/assets/css/reboot.min.css | ||
| cp ./node_modules/bootstrap-reboot/dist/reboot.min.css* ./site/themes/sogilis/assets/css/ | ||
| # netlifh-cms | ||
| cp ./node_modules/netlify-cms/dist/netlify-cms.js* ./site/static/admin/ | ||
| ## fonts | ||
| # fonts | ||
| rm -rf ./site/themes/sogilis/static/css/typeface-montserrat | ||
| cp -R ./node_modules/typeface-montserrat ./site/themes/sogilis/static/css/typeface-montserrat | ||
| rm -rf ./site/themes/sogilis/static/css/typeface-eb-garamond | ||
| cp -R ./node_modules/typeface-eb-garamond ./site/themes/sogilis/static/css/typeface-eb-garamond | ||
| # COPY netlify-cms | ||
| 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/ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. J'ai l'impression que ce PS : idem peut-être pour les autres rm / cp |
||
|
|
||
| _build_common: | ||
| _netlify-build-common: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Qui dit
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Parce que rien n'interdit d'essayer de déployer en local ;-) . Et ça coûte pas cher :-) |
||
| # TESTS to know if repo is clean for build | ||
| # ========================================= | ||
| # Test if we have unstaged changes is not a necessity | ||
| # (`git clean` does not checkout them) but it's a good practice. | ||
| @if ! git diff --quiet --exit-code ; \ | ||
| then \ | ||
| >&2 echo -e "\n\n\\033[4;31mERROR:" \ | ||
| "You have changes not staged for commit" \ | ||
| "(see them thanks \`git status')." \ | ||
| "Use \`git add' to add changes into the git stagging area," \ | ||
| "or remove this changes.\\033[0m\n\n" ; \ | ||
| exit 10 ; \ | ||
| else \ | ||
| echo -e '\nCool, you have no unstaged changes\n' ; \ | ||
| 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 ]] ; \ | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jibidus je ne suis pas persuadé que ce soit cool qu'à chaque Pour le build, ça me paraît être une bonne pratique. Mais pour Non ?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pourquoi on aurait besoin de supprimer des fichiers sur un
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah mince .
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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é). |
||
| then \ | ||
| >&2 echo -e "\n\n\\033[4;31mERROR:" \ | ||
| "You have untracked files" \ | ||
| "not added into '.gitignore' (see them thanks \`git status')." \ | ||
| "Use \`git add' to add files into the git stagging area," \ | ||
| "or remove this files.\\033[0m\n\n" ; \ | ||
| exit 11 ; \ | ||
| else \ | ||
| echo -e "\nCool, you have no untracked files not included" \ | ||
| "into '.gitignore'\n" ; \ | ||
| fi | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 ?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Je serais intétéressé de comprendre pourquoi tu trouves cela compliqué ? C'est du Bash et du Makefile.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah mince, pour moi ça va. D'autant plus que j'ai la coloration syntaxique ;-) .
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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é.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| # We dont check if we have staged and not commited changes. | ||
| make clean | ||
| make _common | ||
| yarn lint | ||
| yarn prettier:check | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -254,10 +254,11 @@ footer { | |||||
| background-color: var(--light-electric-blue); | ||||||
| margin-left: -1.5em; | ||||||
|
|
||||||
| /* On boostrap, for body line-height: 1.5 */ | ||||||
| /* We have 1.3 + 0.2 = 1.5 */ | ||||||
| /* 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 */ | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oupse ! Merci pour ta vigilence. |
||||||
| /* 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; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Out of scope] On pourrait utiliser des variables pour exprimer une bonne partie de ce qui est en commentaire.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bien sûr ! Oui y'a des variables en CSS. Je note d'améliorer ce point |
||||||
| margin-top: 0.9em; | ||||||
| } | ||||||
|
|
||||||
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.
[next time] le mot
Targetn'apporte pas d'info (on sait qui est dans unMakefile, donc qu'on est en face d'untaget). Cf Poorly Written Comment.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.
Ah ok merci pour l'info !