Roy Golan has posted comments on this change.

Change subject: core: Introduce CDI for Commands dependencies
......................................................................


Patch Set 8:

(3 comments)

....................................................
File backend/manager/modules/bll/pom.xml
Line 140:      <dependency>
Line 141:         <groupId>javax.enterprise</groupId>
Line 142:         <artifactId>cdi-api</artifactId>
Line 143:         <scope>provided</scope>
Line 144:      </dependency>
I'll added it if you think this is necessary. I bet it is transitive otherwise 
I don't see how my project compiled.

can you explain why it is needed?
Line 145:   </dependencies>
Line 146: 
Line 147:   <build>
Line 148: 


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/Backend.java
Line 99:     private static boolean firstInitialization = true;
Line 100:     private String poolMonitoringJobId;
Line 101: 
Line 102:     public Backend() {
Line 103:     }
I'll remove it, this is not for now.  it will be in use in the patch that 
removes all EJB from our beans and moves them to managed-beans

generally a "managed-bean" (i.e the CDI container recognize) must have either 
* no args constructor or
* a constructor annotated @Inject

 Class SomeStorageManager

 @Inject
 public Storage(StorageHelper helper) { ... }
Line 104: 
Line 105:     public static BackendInternal getInstance() {
Line 106:         return EjbUtils.findBean(BeanType.BACKEND, 
BeanProxyType.LOCAL);
Line 107:     }


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
Line 130: 
Line 131:     protected Log log = LogFactory.getLog(getClass());
Line 132: 
Line 133:     @Inject
Line 134:     protected DbFacade dbFacade;
setter for sure not. why? 
getter isn't needed as well because we don't need the encapsulation and to 
expose it to external users of the cmd. - unless I'm missing something?
I'd expect clients of commands to be able to lay they're hands on a DbFacade 
instance by an injected field as well.
Line 135: 
Line 136:     /** The context defines how to monitor the command and handle its 
compensation */
Line 137:     private final CommandContext context = new CommandContext();
Line 138: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7f604ff91847b698efe84a09f724ba0492a672c1
Gerrit-PatchSet: 8
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Roy Golan <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Asaf Shakarchi <[email protected]>
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Laszlo Hornyak <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Mike Kolesnik <[email protected]>
Gerrit-Reviewer: Moti Asayag <[email protected]>
Gerrit-Reviewer: Roy Golan <[email protected]>
Gerrit-Reviewer: Sergey Gotliv <[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

Reply via email to