Yair Zaslavsky has posted comments on this change.
Change subject: engine: Add support for IBM Tivoli Directory Server.
......................................................................
Patch Set 1: (6 inline comments)
Sharad,
First of all - I would like to congratulate you on the work you did.
You did a great job here.
I wrote some minor comments, nothing too serious.
Functionality-wise this looks fine to me.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/GetRootDSE.java
Line 123: } else if
(vendorName.equals(LdapVendorNameEnum.IBMVendorName.getName())) {
Sharad , a question here - is Tivoli DS the only directory service product IBM
has?
Maybe TivoliDSVendorName will be more appropriate? As you can see, we used RHDS
and IPA which are product names (maybe VendorName is misleading, a note to
myself).
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/IBMDSGroupAttributes.java
Line 3: public enum IBMDSGroupAttributes {
Once again, same comment as before (this is just a suggestion) - the reason we
used RHDS is because there is DS by that name. In my humble opinion, this
should be TivoliDSGroupAttributes.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/IBMDSRootDSEContextMapper.java
Line 32: // TODO Auto-generated catch block
Please remove this comment.
Line 33: e.printStackTrace();
Please use our logger here.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapProviderType.java
Line 7: ibm,
I promise this is last time I repeat this comment :)
As I suggested - add here Tivoli instead of IBM , if the official product name
is Tivoli (as you can see, we added here other product names).
....................................................
File
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ipa/SimpleAuthenticationCheck.java
Line 95: String query = "";
In general, it's hard to follow changes when formatting was introduced.
I would recommend to try to avoid this.
You can set in Eclipse IDE to change formatting only of changed parts upon
saving.
--
To view, visit http://gerrit.ovirt.org/4633
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iffe980022787936fc79370104bb7f5c1129f0ce5
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Sharad Mishra <snmis...@linux.vnet.ibm.com>
Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com>
Gerrit-Reviewer: Roy Golan <rgo...@redhat.com>
Gerrit-Reviewer: Ryan Harper <ry...@us.ibm.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
