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 <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Arik Hadas <[email protected]>
Gerrit-Reviewer: Doron Fediuck <[email protected]>
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Kobi Ianko <[email protected]>
Gerrit-Reviewer: Lior Vernia <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
