-
Notifications
You must be signed in to change notification settings - Fork 4
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
Feature/improve contributor xp #8
base: master
Are you sure you want to change the base?
Feature/improve contributor xp #8
Conversation
This PR is ready to be reviewed :) Ping @arnaudforaison @samuel-gomez @guillaumechervet @xballoy @youf-olivier |
8fac0f0
to
dfdd031
Compare
|
||
Keep in mind, though, that your pull request will be squashed into master, so repository mainteners may use your commit message but are not entitled to use them as is. | ||
|
||
## Scripts |
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.
Le détails des scripts ne me semblent pas utile ici. Le développeur a l'habitude de les trouver dans le package.json. @samuel-gomez-axa ton avis ?
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.
perso, je trouve ça pas mal d'expliquer la fonction de chaque script, faudra juste veiller à maintenir la doc
README.md
Outdated
|
||
**If you've previously installed gulp globally**, `run npm rm --global gulp` before following these instructions. For more information, read this [Sip][https://medium.com/gulpjs/gulp-sips-command-line-interface-e53411d4467] | ||
This website is the [react-toolkit](https://github.com/AxaGuilDEv/react-toolkit) documentation. |
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.
Plus design system que toolkit ?
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.
Non, justement, l'idée est de dire qu'on affiche ici la documentation en lien avec le toolkit.
Je peux tourner la phrase différemment:
This design system is implemented by the react-toolkit
Est-ce plus clair?
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'ai pushé un fix. A vous de me dire.
README.md
Outdated
|
||
#### Check for Node and npm | ||
Each time a component is updated, it should be documented here. |
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.
Pas tout a fait d'accord car le design system est le point d'entré et le toolkit l'implémentation de ce design system. Un nouveau composant doit être documenté avant d'être implémenter. C'est pour cela qu'il n'y a pas de lien design-system vers toolkit.
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.
Parfait, ça me va!
Sauf, justement un lien pour dire que le react-toolkit implémente ce design system, c'est une information importante.
Si un jour un autre toolkit vient l'implémenter (e.g. svelte-toolkit ou autre), on pourra rajouter le lien vers cette nouvelle implémentation. Qu'en pensez-vous?
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'ai pushé un fix. A vous de me dire.
Fixing issue #7
Adding:
Also fixing this annoying issue when trailing whitespaces are trimmed in .pug files...