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

Feature/improve contributor xp #8

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

gcruchon
Copy link

Fixing issue #7

Adding:

  • CONTRIBUTING.md
  • ISSUE_TEMPLATE.md
  • CODE_OF_CONDUCT.md
  • PULL_REQUEST_TEMPLATE.md
  • LICENSE

Also fixing this annoying issue when trailing whitespaces are trimmed in .pug files...

@gcruchon
Copy link
Author

This PR is ready to be reviewed :)

Ping @arnaudforaison @samuel-gomez @guillaumechervet @xballoy @youf-olivier

@gcruchon gcruchon force-pushed the feature/improve-contributor-xp branch from 8fac0f0 to dfdd031 Compare October 25, 2020 19:48
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved

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

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 ?

Copy link
Contributor

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.

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 ?

Copy link
Author

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?

Copy link
Author

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.

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.

Copy link
Author

@gcruchon gcruchon Oct 31, 2020

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?

Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants