[doc] Add ADR about services organization#1592
Conversation
adaussy
left a comment
adaussy
left a comment
There was a problem hiding this comment.
It is just remarks that need to be discussed.
|
|
||
| - Services that query/mutate the model will be located in the new module `syson-sysml-metamodel-services`. | ||
| - this module will depend on `syson-sysml-metamodel`. | ||
| - all services in this module won't depend on Sirius-Web or any diagram/table related code. |
There was a problem hiding this comment.
I would make this coment not dependend of Spring, so it can be reused in other context.
| - all services in this module won't depend on Sirius-Web or any diagram/table related code. | ||
| - Services that query/mutate the model through Sirius-Web services will be located in the existing module `syson-services`. | ||
| - all services in this module will be allowed to depend on Sirius-Web | ||
| - for example, services that query the model through `IObjectSearchService`. |
There was a problem hiding this comment.
I guess this where I would plug the to previous services in Spring system.
| - Services that query/mutate a tree will be located in the new module `syson-tree-services`. | ||
| - all services in this module will be allowed to depend on Sirius-Web. | ||
| - Services that query/mutate a form will be located in the new module `syson-form-services`. | ||
| - all services in this module will be allowed to depend on Sirius-Web. |
There was a problem hiding this comment.
I'm not sure if all those modules will not make the creation of new "common" service harder.
What I really like is to create only one "Sirius" Service class by representation. This is the main entry point so devellpers always know where to start debuging (or create a new service for a representation). It give the correct place to put code that only impact one representation. For any other comment behavior I whould create a service in syson-service and then inject then in the Sirius class services.
I would also create a naming convention to easily distinguish "Sirius" services class from other "Spring" service.
| - a service querying label of elements will be in a class named `LabelQueryService`. | ||
| - a service modifying the label of elements will be in a class named `LabelMutationService`. | ||
| - a service querying label of diagram elements will be in a class named `LabelDiagramQueryService`. | ||
| - a service modifying the label of elements will be in a class named `LabelDiagramMutationService`. |
There was a problem hiding this comment.
I'm not sure to create as many service class, it could be great to split some of them between semantic but 8 might be too much. I think we need to ease the creation/update of service. Having to many choice might lead to complixity.
For DiagramServices, I would have one for each representation (maybe for each view type if we see it makes to many of them for the "GeneralViewDiagram" an maybe a CommonDiagramService. But I'm not sure it is usefull to split between Query/mutation.
I would not create a specific service for label. The label might be different en each representation (Tree, Table, General view, as IconLabel, as container). I would put them in each reprensetation class service, splitting query and mutation in two different methods. Then it might be delegatted in a common service.
| - a service modifying the label of elements will be in a class named `LabelDiagramMutationService`. | ||
|
|
||
|
|
||
| - The initial categories of services will be: |
There was a problem hiding this comment.
Is the "category" the "something" in previous paragraphe? If it is does, it might induce a really big number of service.
| - Node | ||
| - Edge | ||
| - BorderNode | ||
| - Compartment |
There was a problem hiding this comment.
Not sure that we really need to distinguish NodeType BorderNode et Compartment. Once service class with several methods.
| - `syson-table-services` | ||
| - Row | ||
| - Column | ||
| - Cell |
There was a problem hiding this comment.
For me those services will be stored in each representation service let say RequirementTableService (and maybe in a CommonTableService if we need to mutualize some code at some point). Again using the previously discuss pattern of one "Sirius" service class by representation.
| - Cell | ||
| - `syson-tree-services` | ||
| - Tree | ||
| - TreeItem |
| - Tree | ||
| - TreeItem | ||
| - `syson-form-services` | ||
| - Widget |
| - `syson-form-services` | ||
| - Widget | ||
| - `syson-representation-services` | ||
| - Expose |
There was a problem hiding this comment.
Humm I guess it would also be accessible throught the service defined for each representation but for this one, I guess we will really need a "CommonExposeService".
AxelRICHARD
force-pushed
the
ari/doc/adr002
branch
from
3d37fbe to
183d4eb
Compare
AxelRICHARD
force-pushed
the
ari/doc/adr002
branch
2 times, most recently
from
1ca7e5b to
79c00f0
Compare
AxelRICHARD
added
pr: to review soon
priority: high
and removed
type: architecture
package: services
labels
Signed-off-by: Axel RICHARD <axel.richard@obeo.fr>
AxelRICHARD
force-pushed
the
ari/doc/adr002
branch
4 times, most recently
from
25ad2e5 to
6521c2a
Compare
Bug: #1628 Signed-off-by: Axel RICHARD <axel.richard@obeo.fr>
AxelRICHARD
force-pushed
the
ari/doc/adr002
branch
from
6521c2a to
1880172
Compare
2 participants
PLEASE READ ALL ITEMS AND CHECK ONLY RELEVANT CHECKBOXES BELOW
Project management
priority:andpr:labels been added to the pull request? (In case of doubt, start with the labelspriority: lowandpr: to review later)area:,type:)Changelog and release notes
CHANGELOG.adoc+doc/content/modules/user-manual/pages/release-notes/YYYY.MM.0.adocbeen updated to reference the relevant issues?CHANGELOG.adoc+doc/content/modules/user-manual/pages/release-notes/YYYY.MM.0.adoc?doc/content/modules/user-manual/pages/release-notes/YYYY.MM.0.adoc?Key highlightssection indoc/content/modules/user-manual/pages/release-notes/YYYY.MM.0.adoc?CHANGELOG.adoc+doc/content/modules/user-manual/pages/release-notes/YYYY.MM.0.adoc?Documentation
Tests