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

feat: add poo/value_objects #15

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

Conversation

Emalios
Copy link
Contributor

@Emalios Emalios commented Aug 9, 2020

No description provided.

Copy link
Member

@LukaMrt LukaMrt left a comment

Choose a reason for hiding this comment

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

Les exemples sont très bons et l'explication est globalement bien aussi. Quelques typos à corriger.
En revanche il y a pour moi une erreur d'explication au niveau des dépendances sur la fin.
A revoir

poo/value_objects/README.md Outdated Show resolved Hide resolved
poo/value_objects/code/Money.java Outdated Show resolved Hide resolved
poo/value_objects/fr/VALUE_OBJECTS.MD Outdated Show resolved Hide resolved
poo/value_objects/fr/VALUE_OBJECTS.MD Outdated Show resolved Hide resolved
poo/value_objects/fr/VALUE_OBJECTS.MD Outdated Show resolved Hide resolved
poo/value_objects/fr/VALUE_OBJECTS.MD Outdated Show resolved Hide resolved
Copy link
Contributor Author

@Emalios Emalios left a comment

Choose a reason for hiding this comment

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

Resolve all requests

LukaMrt
LukaMrt previously approved these changes Aug 10, 2020
LukaMrt
LukaMrt previously approved these changes Aug 10, 2020
Copy link
Member

@Tran-Antoine Tran-Antoine left a comment

Choose a reason for hiding this comment

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

Bonnes explications !
Seule remarque, je trouve un peu dommage de ne pas mettre en avant les avantages des value objects. Ils sont bien définis, expliqués et exemplifiés, mais tu ne dis nulle part à quoi ils servent, en gros pourquoi éviter la primitive obsession. Peut-être un paragraphe supplémentaire à ajouter dans une future PR ?

poo/value_objects/README.md Outdated Show resolved Hide resolved
poo/value_objects/fr/VALUE_OBJECTS.MD Outdated Show resolved Hide resolved
poo/value_objects/fr/VALUE_OBJECTS.MD Outdated Show resolved Hide resolved
poo/value_objects/fr/VALUE_OBJECTS.MD Outdated Show resolved Hide resolved
poo/value_objects/fr/VALUE_OBJECTS.MD Outdated Show resolved Hide resolved
Copy link
Contributor Author

@Emalios Emalios left a comment

Choose a reason for hiding this comment

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

Reviewing changes

Copy link
Contributor Author

@Emalios Emalios left a comment

Choose a reason for hiding this comment

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

Review changes

@AntoineJT AntoineJT changed the title Add Value objects notion (fr) feat: add poo/value_objects Aug 12, 2020
Copy link
Member

@AntoineJT AntoineJT left a comment

Choose a reason for hiding this comment

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

Il y a quelques trucs qui me paraissent bizarres :

  • le fait que tu parles de 2 caractéristiques principales mais que le document soit centré autour de 3 caractéristiques ;
  • la conclusion qui n'est qu'un regroupement des 3 caractéristiques.
    Je pense que pour la conclusion tu pourrais vite fait expliquer à quoi sert un value object en définitive, un peu comme un tl;dr quoi.

@LukaMrt LukaMrt added the enhancement New feature or request label Aug 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants