Omer Frenkel has posted comments on this change.

Change subject: core, restapi : Add /tags sub-collection for Template resource
......................................................................


Patch Set 5: Code-Review+1

(1 comment)

thanks! looks good, just one minor comment

http://gerrit.ovirt.org/#/c/24577/5/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AttachTemplatesToTagCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AttachTemplatesToTagCommand.java:

Line 25:     @Override
Line 26:     protected void executeCommand() {
Line 27:         for (Guid templateGuid : getTemplatesList()) {
Line 28:             VmTemplate template = 
DbFacade.getInstance().getVmTemplateDao().get(templateGuid);
Line 29:             if 
(DbFacade.getInstance().getTagDao().getTagTemplateByTagIdAndByTemplateId(getTagId(),
 templateGuid) == null) {
looks like in any case we are checking if template is null, and if it does we 
are doing nothing, so better to check it once before going to the db to get the 
tag
Line 30:                 if (template != null) {
Line 31:                     appendCustomValue("TemplatesNames", 
template.getName(), ", ");
Line 32:                     TagsTemplateMap map = new 
TagsTemplateMap(getTagId(), templateGuid);
Line 33:                     
DbFacade.getInstance().getTagDao().attachTemplateToTag(map);


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I32e2d449f7154c50bc7511e143ed82c995a3fa4e
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ravi Nori <[email protected]>
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Ravi Nori <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: [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

Reply via email to