Arik Hadas has posted comments on this change.
Change subject: userportal,webadmin: implement VM builder
......................................................................
Patch Set 8: (5 inline comments)
the comments at HaVmToVmBuilder.java & QuotaUnitToVmBuilder.java are relevant
to other places where there are 'frontend' and 'backend' as well
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/builders/vm/HaVmToVmBuilder.java
Line 5:
Line 6: public class HaVmToVmBuilder extends BaseSyncBuilder<VM, VM> {
Line 7:
Line 8: @Override
Line 9: protected void build(VM frontend, VM backend) {
why not using source and destination for consistency?
Line 10: backend.setDedicatedVmForVds(frontend.getDedicatedVmForVds());
Line 11: backend.setMigrationSupport(frontend.getMigrationSupport());
Line 12: }
Line 13:
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/builders/vm/QuotaUnitToVmBuilder.java
Line 7:
Line 8: public class QuotaUnitToVmBuilder extends BaseSyncBuilder<UnitVmModel,
VM> {
Line 9:
Line 10: @Override
Line 11: protected void build(UnitVmModel frontend, VM backend) {
same here
Line 12: if (frontend.getQuota().getSelectedItem() != null) {
Line 13: backend.setQuotaId(((Quota)
frontend.getQuota().getSelectedItem()).getId());
Line 14: }
Line 15: }
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/builders/vm/VmSpecificUnitToVmBuilder.java
Line 9:
Line 10: @Override
Line 11: protected void build(UnitVmModel frontend, VM backend) {
Line 12:
Line 13: String name = (String) frontend.getName().getEntity();
can be inlined
Line 14:
Line 15: VmTemplate template = (VmTemplate)
frontend.getTemplate().getSelectedItem();
Line 16: backend.setVmtGuid(template.getId());
Line 17: backend.setName(name);
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/userportal/UserPortalListModel.java
Line 990: new CommonUnitToVmBuilder(),
Line 991: new KernelParamsUnitToVmBuilder(),
Line 992: new MigrationSupportVmToUnitBuilder(),
Line 993: new QuotaUnitToVmBuilder(),
Line 994: new UsbPolicyUntiToVmBuilder(),
nice
Line 995: new VmSpecificUnitToVmBuilder()).build(model,
gettempVm());
Line 996:
Line 997: if (model.getIsNew())
Line 998: {
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmListModel.java
Line 1386:
Line 1387: VM newVm = new VM();
Line 1388: new BuilderExecutor<VM, VM>(
Line 1389: new KernelParamsAndIdVmToVmBuilder(),
Line 1390: new HaVmToVmBuilder(),
wouldn't it be better to give the elements in the builder constructors and
change the signature of the 'build' method to be with no parameters? that way
you'll manage to use one executor, what do you think?
Line 1391: new UsbPolicyVmToVmBuilder()).build(vm, newVm);
Line 1392:
Line 1393: new BuilderExecutor<UnitVmModel, VM>(
Line 1394: new CommonUnitToVmBuilder(),
--
To view, visit http://gerrit.ovirt.org/13915
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I54d938896e469c73800297c3d66185a439243a41
Gerrit-PatchSet: 8
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Tomas Jelinek <[email protected]>
Gerrit-Reviewer: Alona Kaplan <[email protected]>
Gerrit-Reviewer: Arik Hadas <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
