[2194] Properly report feedback messages while moving elements in GV#2217
[2194] Properly report feedback messages while moving elements in GV#2217adaussy wants to merge 1 commit into
Conversation
Bug: #2194 Signed-off-by: Arthur Daussy <arthur.daussy@obeo.fr>
There was a problem hiding this comment.
Is it mandatory to create a new .sql file?
Please consider with the greatest attention the need of such file, may be there are existing ones that could suit your needs.
Every new .sql is a step into a more complex integration tests suite to maintain.
|
|
||
| private final DiagramQueryElementService diagramQueryElementService; | ||
|
|
||
| private final IFeedbackMessageService feedbackMessageService; |
There was a problem hiding this comment.
I would prefer you add the two SW services in first positions, here, in the constructor parameters and constructor body.
| } | ||
|
|
||
| private String getLabel(Object droppedElement) { | ||
| StyledString styledLabel = this.labelService.getStyledLabel(droppedElement); |
There was a problem hiding this comment.
Is it really necessary to invoke getStyledLabel here?
| private String getLabel(Object droppedElement) { | ||
| StyledString styledLabel = this.labelService.getStyledLabel(droppedElement); | ||
| if ((styledLabel == null || styledLabel.toString().isEmpty()) && droppedElement instanceof EObject droppedEObject) { | ||
| styledLabel = StyledString.of(droppedEObject.eClass().getName()); |
There was a problem hiding this comment.
No need to String => StyledString => String.
You should probably create a String variable as the first line of this method body, then affect droppedEObject.eClass().getName() to it.
| private String getLabel(Object droppedElement) { | ||
| StyledString styledLabel = this.labelService.getStyledLabel(droppedElement); | ||
| if ((styledLabel == null || styledLabel.toString().isEmpty()) && droppedElement instanceof EObject droppedEObject) { | ||
| styledLabel = StyledString.of(droppedEObject.eClass().getName()); |
There was a problem hiding this comment.
No need to String => StyledString => String.
You should probably create a String variable as the first line of this method body, then affect droppedEObject.eClass().getName() to it.
| } | ||
|
|
||
| private String getLabel(Object droppedElement) { | ||
| StyledString styledLabel = this.labelService.getStyledLabel(droppedElement); |
There was a problem hiding this comment.
Is it really necessary to invoke getStyledLabel here?
2 participants
Bug: #2194
PLEASE READ ALL ITEMS AND CHECK ONLY RELEVANT CHECKBOXES BELOW
Auto review
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?CHANGELOG.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?Documentation
Tests