Dudi Maroshi has posted comments on this change.

Change subject: restapi: add field ksm_merge_across_node to cluster
......................................................................


Patch Set 2:

(1 comment)

https://gerrit.ovirt.org/#/c/39783/2/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/ClusterMapper.java
File 
backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/ClusterMapper.java:

Line 107:             entity.setEnableKsm(model.getKsm().isEnabled());
Line 108:         }
Line 109:         if (model.isSetKsmMergeAcrossNodes() && 
model.getKsmMergeAcrossNodes().isSetEnabled()) {
Line 110:             
entity.setKsmMergeAcrossNumaNodes(model.getKsmMergeAcrossNodes().isEnabled());
Line 111:         }
> Consider creating a new "KSMMapper" class and move all the related code the
I thought about writing a mapper class for my toggle.
And I abandoned it for the following reason.

1. It will not save clutter on this class, since there are at least 2 
conversion calls in the ClusterMapper.java class. 

2. Saving 2 lines of code in ClusterMapper.java class, produces 12 lines of 
codes in KsmMapper.java (2 constructor + 4 accessors + 4 convertors). Which 
will require maintenance (migrations and upgrades).
Line 112:         if (model.isSetDisplay() && model.getDisplay().isSetProxy()) {
Line 113:             
entity.setSpiceProxy("".equals(model.getDisplay().getProxy()) ? null : 
model.getDisplay().getProxy());
Line 114:         }
Line 115:         if (model.isSetSerialNumber()) {


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idd6ce2c42b4d7fa30088c77d5f16d0dfdabb156a
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dudi Maroshi <d...@redhat.com>
Gerrit-Reviewer: Dudi Maroshi <d...@redhat.com>
Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com>
Gerrit-Reviewer: Roy Golan <rgo...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to