Roy Golan has posted comments on this change. Change subject: core: Introduce CDI for Commands dependencies ......................................................................
Patch Set 13: (2 comments) I have two comments: A.) I could not understand why we need BllCDIAdapter? Isn't it better to put @Produces in the dependent class? That way we have @Produces where we create something, and @Inject where we need it. If we need to update this class when ever we want to add a new dependency to be injected in bll it goes against adding a managed infra? B.) Regarding injector, Another option is to inject needed components in the injecting class (CommandsFactory here) And inject them manually. thus avoiding mixing managed and non managed environments. To summarize I think we need as lees infra around CDI as possible, on the other hand I'm sure you looked into it in depth so you probably see problems I do not. please explain those. @Mooli I should probably squash this patch with the next one. BllCDIAdapter have been made to brige Jboss gap where it couldn't identify classes from other modules. this has been removed in the next patch. I guess I wanted to preserve the history of this patch for the matter of the review - I'll squash it and abandon this and put a link to this patch for the review history http://gerrit.ovirt.org/#/c/5575/13/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java: Line 306: (); > So the default behavior is to assume injected beans are != There is never a situation where the container injects you a null instance! on startup, the container makes sure it can fulfill dependencies at all the injection points. if it can't it throws an exception Line 833: ActionVersionMap > remove variable? I want to minimize the patch scope. its already "small" -- 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: 13 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: Liran Zelkha <[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: Ravi Nori <[email protected]> Gerrit-Reviewer: Roy Golan <[email protected]> Gerrit-Reviewer: Sergey Gotliv <[email protected]> Gerrit-Reviewer: Tal Nisan <[email protected]> Gerrit-Reviewer: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: mooli tayer <[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
