Juan Hernandez has posted comments on this change.
Change subject: WIP: working CDI solution for backend
......................................................................
Patch Set 4: Looks good to me, but someone else must approve
(5 inline comments)
Some minor comments, except the comment about the maven dependencies, which I
think it is important (but not super important).
I am changing the POMs as suggested and doing a full build to verify.
....................................................
File backend/manager/modules/bll/pom.xml
Line 167: <groupId>javax.enterprise</groupId>
Line 168: <artifactId>cdi-api</artifactId>
Line 169: <version>1.0-SP2</version>
Line 170: <scope>provided</scope>
Line 171: </dependency>
Can you add this to the root POM?
<properties>
...
<cdi.version>1.0-SP2</cdi.version>
</prperties>
<dependencyManagement>
<dependencies>
...
<dependency>
<groupId>javax.enterprise</groupId>
<artifactId>cdi-api</artifactId>
<version>${cdi.version}</version>
</dependency>
</dependencies>
</dependencyManagement>
Then in this POM you don't need to specify the version.
Line 172:
Line 173: <dependency>
Line 174: <groupId>org.jboss.weld</groupId>
Line 175: <artifactId>weld-se</artifactId>
Line 175: <artifactId>weld-se</artifactId>
Line 176: <version>1.0.1-Final</version>
Line 177: <type>jar</type>
Line 178: <scope>provided</scope>
Line 179: </dependency>
You don't need to make this dependency explicity, as the code doesn't depend on
any weld specif API.
Line 180:
Line 181: <dependency>
Line 182: <groupId>org.jboss.solder</groupId>
Line 183: <artifactId>solder-impl</artifactId>
Line 181: <dependency>
Line 182: <groupId>org.jboss.solder</groupId>
Line 183: <artifactId>solder-impl</artifactId>
Line 184: <version>3.1.1.Final</version>
Line 185: </dependency>
Same here, no need to make this explicit.
Line 186:
Line 187: </dependencies>
Line 188:
Line 189: <build>
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
Line 1367: @Inject
Line 1368: private BackendInternal backend;
Line 1369:
Line 1370: @Inject
Line 1371: DbFacade db;
Any reason to not make this protected?
Line 1372:
Line 1373: protected EngineLock getLock() {
Line 1374: return commandLock;
Line 1375: }
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InjectionHelper.java
Line 11: private BeanManager manager;
Line 12:
Line 13: public void setManager(BeanManager manager) {
Line 14: this.manager = manager;
Line 15: }
This setter is not used, do you have any plan to use it in the future?
Line 16:
Line 17: public void bind(CommandBase<?> command) {
Line 18: AnnotatedType type =
manager.createAnnotatedType(command.getClass());
Line 19: manager.createInjectionTarget(type).inject(command,
manager.createCreationalContext(null));
--
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: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Roy Golan <rgo...@redhat.com>
Gerrit-Reviewer: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com>
Gerrit-Reviewer: Laszlo Hornyak <lhorn...@redhat.com>
Gerrit-Reviewer: Roy Golan <rgo...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
