Alon Bar-Lev has posted comments on this change.

Change subject: 8. [WIP] core: Introducing the extension interface
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.ovirt.org/#/c/24811/1/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/Directory.java
File 
backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/Directory.java:

Line 10: /**
Line 11:  * A directory is an object that manages a collection of users and 
groups, usually stored in an external system like an
Line 12:  * LDAP database.
Line 13:  */
Line 14: public abstract class Directory implements Extension, Serializable {
> 1. we can remove Serializable, now that it's not in common, and not being u
1. I would like to see both classes with same declaration, there should be no 
difference one way or another.

2. Hmm... I do not like abstract classes, we will try to avoid these when 
publish interface... so keep it this way, but see what I recommend for 
interface - only getAttribute/setAttribute.
Line 15:     /**
Line 16:      *
Line 17:      */
Line 18:     private static final long serialVersionUID = -8724317446083142917L;


http://gerrit.ovirt.org/#/c/24811/1/backend/manager/modules/extension-manager/src/main/java/org/ovirt/engine/core/extensions/mgr/Extension.java
File 
backend/manager/modules/extension-manager/src/main/java/org/ovirt/engine/core/extensions/mgr/Extension.java:

Line 36:      * Gets the configuration properties
Line 37:      *
Line 38:      * @return the configuration properties
Line 39:      */
Line 40:     Properties getConfigurationProperties();
> Why would I want that? I would like to work with Properties for the configu
well... I suggest a different interface, closer to what we are going to have...

 setProperty(enum, Object)
 Object getProperty(enum)

our current properties are: Name, Configuration, Version, Author, License, 
Extension Name, Interface Version (1).
Line 41: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8f583d635c059972a2a536d3c49a58cfcf3234b
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@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