Omer Frenkel has posted comments on this change.

Change subject: core: Move getFQN to SetVmTicketCommand
......................................................................


Patch Set 2:

(1 comment)

although the change looks ok, oop wise, this method is more related to the user 
object than the command, why moving it?
also, please move the test instead of deleting it, i think its still relevant

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SetVmTicketCommand.java
Line 55:         }
Line 56:         return permissions;
Line 57:     }
Line 58: 
Line 59:     private String getConsoleUserName(VdcUser user) {
since this is a private method of the class no need to sent user as a 
parameter, just use getCurrentUser()
Line 60:         String domain = user.getDomainControler();
Line 61:         String name = user.getUserName();
Line 62:         if (StringUtils.isEmpty(name) || name.contains("@") || 
StringUtils.isEmpty(domain)) {
Line 63:             return name;


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If4a3a45136db9c42ab347f9722326f597afd26fd
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Juan Hernandez <juan.hernan...@redhat.com>
Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourf...@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