Moti Asayag has posted comments on this change.

Change subject: engine: minor CommandBase change for rename logging
......................................................................


Patch Set 1: (3 inline comments)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
Line 547:         if (this instanceof RenamedEntityInfoProvider) {
Line 548:             RenamedEntityInfoProvider renameable = 
(RenamedEntityInfoProvider) this;
Line 549:             String entityType = renameable.getEntityType();
Line 550:             String oldEntityName = renameable.getEntityOldName();
Line 551:             String newEntityName = renameable.getEntityNewName();
please run a formatter for the if block. it is not properly indented
Line 552:             if (!StringUtils.equals(oldEntityName, newEntityName)) {
Line 553:                 // log entity rename details
Line 554:                 AuditLogableBase logable = new AuditLogableBase();
Line 555:                 logable.AddCustomValue("EntityType", entityType);


Line 548:             RenamedEntityInfoProvider renameable = 
(RenamedEntityInfoProvider) this;
Line 549:             String entityType = renameable.getEntityType();
Line 550:             String oldEntityName = renameable.getEntityOldName();
Line 551:             String newEntityName = renameable.getEntityNewName();
Line 552:             if (!StringUtils.equals(oldEntityName, newEntityName)) {
the user of StringUtils.equals was already introduced in the 7th version of the 
dependent patch.
Line 553:                 // log entity rename details
Line 554:                 AuditLogableBase logable = new AuditLogableBase();
Line 555:                 logable.AddCustomValue("EntityType", entityType);
Line 556:                 logable.AddCustomValue("OldEntityName", 
oldEntityName);


....................................................
File 
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CommandBaseTest.java
Line 191:             }
Line 192: 
Line 193:         }
Line 194:         RenameCommand command = Mockito.mock(RenameCommand.class);
Line 195:         Mockito.when(command.getEntityOldName()).thenReturn(null);
would you consider declaring mockito package with static imports as commonly 
used within tests ?
Line 196:         Mockito.when(command.getEntityNewName()).thenReturn(null);
Line 197:         Mockito.doCallRealMethod().when(command).logRenamedEntity();
Line 198:         command.logRenamedEntity();
Line 199:         Mockito.when(command.getEntityOldName()).thenReturn("foo");


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8b437192af2635a133d1636c2ede39eb91a51c71
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak <lhorn...@redhat.com>
Gerrit-Reviewer: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Moti Asayag <masa...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to