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/Integration de SPI #156

Closed
wants to merge 18 commits into from
Closed

Conversation

saveliy-sviridov
Copy link
Collaborator

No description provided.

@saveliy-sviridov saveliy-sviridov added the WIP Work In Progress: the branch is still under active development and not stable / in a working state label Aug 7, 2024
Copy link
Collaborator

@romainfd romainfd left a comment

Choose a reason for hiding this comment

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

Je pense qu'on va dans la bonne direction et c'est vraiment top comme travail ! Deux grosses interrogations de mon côté :

  • pourquoi ne pas remonter l'EDXL complet (hors ContentMessage) directement dans la lib d'interface ?
  • à quoi servent les interfaces vides par rapport à des Object ou JsonNode ?

}

dependencies {
implementation 'org.springframework.boot:spring-boot-starter-amqp:2.7.18'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pourquoi models-interface dépend d'amqp ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok pour AbstractHubException extends AmqpRejectAndDontRequeueException j'ai l'impression ! Et pourquoi on a besoin de cet extend ? C'est pour avoir un seul catch côté Dispatcher ?

@@ -24,5 +24,4 @@ public class Constants {
public static final String TECHNICAL_SCHEMA = "TECHNICAL.schema.json";
public static final String TECHNICAL_NOREQ_SCHEMA = "TECHNICAL_NOREQ.schema.json";
public static final String TECHNICAL_XSD = "TECHNICAL.xsd";
public static final String TECHNICAL_NOREQ_XSD = "TECHNICAL_NOREQ.xsd";
}
public static final String TECHNICAL_NOREQ_XSD = "TECHNICAL_NOREQ.xsd";}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Plus clean et moins source d'erreur avec le } à la ligne non ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

A quoi servent les interfaces vides ? On peut pas juste utiliser Object ou JsonNode ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Je me demande s'il a besoin d'être interfacé ! Il n'est pas amené à bouger et n'a pas de sens métier fort donc peut-être qu'il pourrait être directement dans l'interface. Sauf si ça rend les tests / usages moins pertinents mais en soit TECHNICAL sert à ça maintenant a priori

Copy link
Collaborator

Choose a reason for hiding this comment

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

Idem, j'ai du mal à comprendre pourquoi on a besoin d'interfaces vides ?

Comment on lines 18 to 23
import com.hubsante.model.edxl.ContentMessage;
import com.hubsante.model.edxl.Descriptor;
import com.hubsante.model.edxl.EdxlMessage;
import com.hubsante.model.edxl.ExplicitAddress;
import com.hubsante.modelsinterface.edxl.DistributionKind;
import com.hubsante.modelsinterface.edxl.DistributionStatus;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ca serait peut-être plus simple d'avoir tout l'edxl dans l'interface non ? Si c'est possible d'en sortir que le CustomContent vers la lib modeles...

@@ -16,7 +16,7 @@
package com.hubsante.model.builders;

import com.hubsante.model.rcde.DistributionElement;
import com.hubsante.model.ValidationMessageWrapper;
import com.hubsante.model.validationmessage.ValidationMessageWrapper;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Peut-être pas super convaincu du nom qui me parait trop spécifique... le mettre dans helpers sinon ? Ou dans Validator.java ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Il sert à quoi ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

A rediscuter mais je pense que ça peut être bougé vers la lib d'interface avec tout l'EDXL

@@ -13,7 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.hubsante.model;
package com.hubsante.model.validationmessage;
Copy link
Collaborator

Choose a reason for hiding this comment

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

cf plus haut sur le nom de ce package

Copy link

sonarcloud bot commented Aug 28, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

@saveliy-sviridov saveliy-sviridov added HOLD The branch is put on hold as it is waiting for something else and removed WIP Work In Progress: the branch is still under active development and not stable / in a working state labels Aug 29, 2024
@romainfd romainfd closed this Nov 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
HOLD The branch is put on hold as it is waiting for something else
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants