Alona Kaplan has posted comments on this change.

Change subject: webadmin: Adding external status to UI
......................................................................


Patch Set 15:

(4 comments)

https://gerrit.ovirt.org/#/c/40999/15/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/widget/table/column/HostAdditionalStatusColumn.java
File 
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/widget/table/column/HostAdditionalStatusColumn.java:

Line 16: import java.util.LinkedHashMap;
Line 17: import java.util.List;
Line 18: import java.util.Map;
Line 19: 
Line 20: public class HostAdditionalStatusColumn<S> extends 
AbstractSafeHtmlColumn<S> {
Why do you need generics here? Isn't it always VDS?
Line 21: 
Line 22:     private final static ApplicationConstants constants = 
AssetProvider.getConstants();
Line 23:     private final static ApplicationResources resources = 
AssetProvider.getResources();
Line 24: 


Line 25: 
Line 26:     @Override
Line 27:     public SafeHtml getValue(S object) {
Line 28:         VDS host;
Line 29:         if (object instanceof VDS){
If you remove the <S> generic, you won't need this if block.
Line 30:             host =  (VDS) object;
Line 31:         }
Line 32:         else {
Line 33:             return null;


Line 48:         return null;
Line 49:     }
Line 50: 
Line 51:     @Override
Line 52:     public SafeHtml getTooltip(S object) {
If you remove the <S> generic, you won't need this if block.
Line 53:         VDS host;
Line 54:         if (object instanceof VDS){
Line 55:             host =  (VDS) object;
Line 56:         }


Line 56:         }
Line 57:         else {
Line 58:             return null;
Line 59:         }
Line 60:         Map<SafeHtml, String> imagesToText = new LinkedHashMap<>();
The 'getTooltip(..)' code is very similar to 'getValue(..)'.
Consider extracting the code in getToolip to a method which constracts 
imagesToText map.
'getValue(..)' method will use just the keys part of the output.
Line 61: 
Line 62:         if (host.isUpdateAvailable()) {
Line 63:             
imagesToText.put(getImageSafeHtml(resources.updateAvailableImage()), 
constants.updateAvailable());
Line 64:         }


-- 
To view, visit https://gerrit.ovirt.org/40999
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I99e7066176e70ec94f543aa1120bb355acad9b0a
Gerrit-PatchSet: 15
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com>
Gerrit-Reviewer: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Moti Asayag <masa...@redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to