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

Starting model #1

Closed
wants to merge 21 commits into from
Closed

Starting model #1

wants to merge 21 commits into from

Conversation

alagane
Copy link
Member

@alagane alagane commented Oct 19, 2018

This is the first class diagram, with relations between classes. Multiplicities in classes are only for max 1 and max * (respectively reference and List).
Each class has getter and setter for each of its attributes.

Copy link
Member

@cdeneux cdeneux left a comment

Choose a reason for hiding this comment

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

  • Don't forget to use modifier final when it is needed,
  • Can you add description of model as javadoc ?

@alagane
Copy link
Member Author

alagane commented Oct 26, 2018

I changed some things in the model, like using maps directly for component parameters and placeholders, thus I removed the description of placeholders.
I used final for setters parameters.
I added Javadoc.

Copy link
Member

@cdeneux cdeneux left a comment

Choose a reason for hiding this comment

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

Minor stuff in POM file

pom.xml Show resolved Hide resolved
pom.xml Outdated Show resolved Hide resolved
Copy link
Member

@cdeneux cdeneux left a comment

Choose a reason for hiding this comment

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

Great job !!

Don't forget to use the modifier final on method parameters, class members and variables.

Remove binary files in directory src/main/resources.

ZIP files in src/test/resources should be smaller as possible. I think that your tests should be ok if they contain only the file jbi.xml.

pom.xml Outdated Show resolved Hide resolved
pom.xml Outdated Show resolved Hide resolved
pom.xml Outdated Show resolved Hide resolved
pom.xml Outdated Show resolved Hide resolved
pom.xml Show resolved Hide resolved
src/main/java/org/ow2/petals/deploymentmodel/Model.java Outdated Show resolved Hide resolved
src/main/java/org/ow2/petals/deploymentmodel/Model.java Outdated Show resolved Hide resolved
@vincent-zurczak
Copy link
Member

Good work indeed.
But you can make your code more compact, more efficient and easier to understand. You also need to homogenize javadoc and line breaks. Also, think about separating beans and utilities in different packages.

@alagane alagane force-pushed the model-creation branch 2 times, most recently from 6550307 to af982d1 Compare January 10, 2019 13:18
Copy link
Member

@cdeneux cdeneux left a comment

Choose a reason for hiding this comment

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

  • Mettre le champ @author de la javadoc en toutes lettres avec le nom de la société: Alexandre LAGANE - Linagora.
  • Ne pas oublier l'usage du modifier final sur les arguments de méthodes, les définitions de variables et les attributs de classes.
  • Dans les modèles XSD, utiliser minOccurs="1" plutôt que de ne pas le mettre. Ca evite la confusion à la lecture au cas où le lecteur ne connait pas le comportement par défaut.

Copy link
Member

@cdeneux cdeneux left a comment

Choose a reason for hiding this comment

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

-- Erreur de localistation pour cette review, à supprimer ou acquitter. Je ne peux pas le faire --

@alagane
Copy link
Member Author

alagane commented Feb 11, 2019

Close to make a clean new one

@alagane alagane closed this Feb 11, 2019
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