Yair Zaslavsky has posted comments on this change.

Change subject: db: Affinity Groups database support
......................................................................


Patch Set 4:

(3 comments)

....................................................
File 
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/BaseDAODbFacade.java
Line 173:     }
Line 174: 
Line 175:     protected static ArrayList<String> split(String str) {
Line 176:         if (StringUtils.isEmpty(str)) {
Line 177:             return null;
IMHO, in case of empty , non null string, should return an array list in side 
of 0.
In addition, any reason this method returns an ArrayList in the signatgure and 
not just list?
I also think you should consider putting such a helper in Utils, so other 
components may enjoy it.
Line 178:         }
Line 179: 
Line 180:         return new 
ArrayList<String>(Arrays.asList(str.split(SEPARATOR)));
Line 181:     }


....................................................
File 
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/scheduling/AffinityGroupDaoImpl.java
Line 80:     }
Line 81: 
Line 82:     @Override
Line 83:     protected RowMapper<AffinityGroup> createEntityRowMapper() {
Line 84:         return new RowMapper<AffinityGroup>() {
I would recommend returning here a static instance. Row mappers are stateless 
by nature.
Line 85: 
Line 86:             @Override
Line 87:             public AffinityGroup mapRow(ResultSet rs, int arg1) throws 
SQLException {
Line 88:                 AffinityGroup affinityGroup = new AffinityGroup();


....................................................
File backend/manager/modules/dal/src/test/resources/fixtures.xml
Line 5564:              </row>
Line 5565:     </table>
Line 5566: 
Line 5567:     <table name="affinity_group_members">
Line 5568:      <column>affinity_group_id</column>
TWS, remove
Line 5569:      <column>vm_id</column>
Line 5570:      <row>
Line 5571:                      
<value>6d849ebf-0ccc-4552-ad09-ccc90cda105d</value>
Line 5572:                      
<value>77296e00-0cad-4e5a-9299-008a7b6f4354</value>


-- 
To view, visit http://gerrit.ovirt.org/22714
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib09c50cae8d9a23a08a9e723d0e0498ea2016e6a
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Gilad Chaplik <gchap...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Arik Hadas <aha...@redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com>
Gerrit-Reviewer: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com>
Gerrit-Reviewer: Kobi Ianko <k...@redhat.com>
Gerrit-Reviewer: Lior Vernia <lver...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
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