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

Refactoring #2

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

Refactoring #2

wants to merge 5 commits into from

Conversation

Trispa
Copy link
Contributor

@Trispa Trispa commented Jun 2, 2017

No description provided.

@Trispa Trispa requested a review from didia June 2, 2017 01:22
Copy link
Member

@didia didia left a comment

Choose a reason for hiding this comment

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

Comme certains de mes commentaires disent, essaie d'installer le hook prettier aussi revois l'organisation des folders et tout

@@ -4,6 +4,7 @@
"private": true,
"dependencies": {
"express": "^4.15.2",
"prop-type": "^0.0.1",
Copy link
Member

Choose a reason for hiding this comment

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

Pourquoi 0.0.1? Alors qu'on est rendu à 15 quelque chose?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

j'ai fai un yarn add et je n'ai pas checké une version en particulier

Copy link
Contributor Author

Choose a reason for hiding this comment

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

effectivement on est a la version 15 mais normalement c;est derniere version compatible qui devrait installé par desfaut so

src/App.js Outdated
@@ -7,8 +7,8 @@ import NavBarItem from "./components/NavBarItem";
import ContentHeader from "./containers/ContentHeader";
import SidebarContent from "./containers/SidebarContent";
import NewUniversity from "./Pages/NewUniversity";
import ViewUniversity from "./Pages/ViewUniversity";
import Universities from "./containers/Universities";
import ViewUniversity from "./containers/university/js/ViewUniversity";
Copy link
Member

Choose a reason for hiding this comment

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

Plutôt que de university/js/ViewUniversity. Si tu as un component pour voir une université, il peut s'appeler containers/University et le component components/University, ensuite dans le dossier tu mettras index.js et index.css

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cela est deja fait si tu regarde le repertoir il ya deja component/university ainsi que container/university.
viewuniversity ne corespon ni a au component ni au container. pour le moment ce qui est fait touche juste les fichier university et universities. les autres fichier contiennenet du code qui va etre bougé dans component/university et component/universties ainsi que leur container.

import If from "../components/If";
import Domains from "../../../containers/Domains";
import If from "../../If";
import '../css/university.css'
Copy link
Member

Choose a reason for hiding this comment

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

Manque un point virgule --> Prettier l'aurait corriger pour toi

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ouinn pas obligatoir en javascript. mais bon je vais installer prettier

divider{
margin: $margin;
height: $height;
backgroundColor: $backgroundColor;
Copy link
Member

Choose a reason for hiding this comment

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

C'est quoi ça?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hum ce fichier ne devrai pas etre dans le PR .. my bad .. je faisait une petit test

Copy link
Member

Choose a reason for hiding this comment

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

Okay Good 👍

Patrice Diouf added 2 commits August 9, 2017 19:47
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.

2 participants