Improve extensibility of model server formats and codecs #60 #61
Conversation
Use Map binding for codecs and format. Also give the possibility to specify a preferred format (json by default). The Model Server Client now has overrideable methods so that it can change the accepteable formats when necessary. Signed-off-by: vhemery <vincent.hemery@csgroup.eu>
@martin-fleck-at Looks like a network temp issue... Can someone try and respin the build ? (or give me a hint on what else is going south) |
|
@vhemery I re-triggered the CI, seems you were just unlucky. Build went through now ;-) |
There was a problem hiding this comment.
Hello @vhemery, thank you very much for your change! I especially like that you considered backwards compatibility and also added some JavaDoc comments ;-) As far as I can tell, everything still works as expected and the tests also run through.
I do have some minor comments which you can incorporate. My main "concern" is that we add a binding based on a class (Codecs), something we aim to get rid of. So I would prefer it if we could add an interface for that and simply bind the Codecs class as the implementation of that interface. What do you think? Of course, this would not prevent this change from merging but it would be change I would probably tackle as part of the DI cleanup.
...cloud.modelserver.emf/src/org/eclipse/emfcloud/modelserver/emf/common/SessionController.java
Outdated
Show resolved
Hide resolved
....emfcloud.modelserver.emf/src/org/eclipse/emfcloud/modelserver/emf/common/codecs/Codecs.java
Show resolved
Hide resolved
....emfcloud.modelserver.emf/src/org/eclipse/emfcloud/modelserver/emf/common/codecs/Codecs.java
Outdated
Show resolved
Hide resolved
....emfcloud.modelserver.emf/src/org/eclipse/emfcloud/modelserver/emf/common/codecs/Codecs.java
Outdated
Show resolved
Hide resolved
....emfcloud.modelserver.emf/src/org/eclipse/emfcloud/modelserver/emf/di/ModelServerModule.java
Show resolved
Hide resolved
...delserver.emf.tests/src/org/eclipse/emfcloud/modelserver/emf/common/ModelControllerTest.java
Outdated
Show resolved
Hide resolved
Changes review : let user the possibility to override Codecs implementation. Signed-off-by: vhemery <vincent.hemery@csgroup.eu>
There was a problem hiding this comment.
Thank you very much @vhemery for the quick update! I re-tested the change and everything still works great and is much more extensible. Great work!
vhemery
deleted the
Improve_extensibility_of_model_server_formats_and_codecs_#60
branch
Use Map binding for codecs and format. Also give the possibility to
specify a preferred format (json by default).
The Model Server Client now has overrideable methods so that it can
change the accepteable formats when necessary.
Signed-off-by: vhemery vincent.hemery@csgroup.eu