Hi Adam, Thanks for working on this. As this is a big diff I'll split the review in multiple passes. Here are a few comments about the upstream code changes (ie outside the debian/ directory): > === modified file 'Makefile' > --- Makefile 2009-12-02 21:04:38 +0000 > +++ Makefile 2010-06-17 16:58:16 +0000 > @@ -43,7 +36,7 @@ > > tarball: clean > mkdir $(NAME)-$(VERSION) > - cp -a Makefile *.sh schemas doc TODO LICENSE COPYRIGHT acls databases overlays modules contents $(NAME)-$(VERSION) > + cp -a Makefile *.sh schemas doc TODO COPYRIGHT acls databases overlays modules contents $(NAME)-$(VERSION) Why is the LICENSE file removed from the tarball? > === added directory 'core' > === added file 'core/core-acl.ldif' > --- core/core-acl.ldif 1970-01-01 00:00:00 +0000 > +++ core/core-acl.ldif 2010-06-17 16:58:16 +0000 > @@ -0,0 +1,34 @@ I would split the acl parts from the database defintion. Every parts that other modules (eg usersandgruops) could define should be part of their own file. The actual database definition could be put in core/database.ldif. > +olcDbDirectory: /var/lib/ldap I would use a sub-directory of /var/lib/ldap/ such as openldap-dit/ so that other packages could also use /var/lib/ldap/ for their own database backend. > +olcDbIndex: objectClass eq > +olcDbIndex: entryUUID eq > +olcDbIndex: entryCSN eq > +olcDbIndex: cn eq,subinitial I would suggest to put all the indexes definition in an core/indexes.ldif file. > +olcDbIndex: uid eq,subinitial > +olcDbIndex: uidNumber eq > +olcDbIndex: gidNumber eq > +olcDbIndex: sn eq,subinitial > +olcDbIndex: member eq > +olcDbIndex: mail eq,subinitial > +olcDbIndex: givenName eq,subinitial > +olcDbIndex: displayName eq > +olcDbIndex: uniqueMember pres,eq All of these indexes are actually related to the user and group module. So move them to usersandgroups/indexes.ldif. > +olcAccess: {0}to dn.subtree="@SUFFIX@" > + by dn.exact="uid=LDAP Admin,ou=System Accounts,@SUFFIX@" manage > + by * break > +olcAccess: {1}to dn.subtree="@SUFFIX@" > + by * read I'd suggest that all the acl definition should go in the core/acl.ldif file. > +olcAddContentAcl: TRUE > +olcLastMod: TRUE These should be part of the core database configuration in core/database.ldif. > === added file 'core/core-dit.ldif' > --- core/core-dit.ldif 1970-01-01 00:00:00 +0000 > +++ core/core-dit.ldif 2010-06-17 16:58:16 +0000 I would name the file dit.ldif instead of core-dit.ldif since core is already in the directory name. > === added file 'core/core-modules.ldif' > --- core/core-modules.ldif 1970-01-01 00:00:00 +0000 > +++ core/core-modules.ldif 2010-06-17 16:58:16 +0000 I would rename core-modules to modules.ldif since core is already part of the directory name. > @@ -0,0 +1,9 @@ > +dn: cn=module,cn=config [...] > +olcModuleLoad: back_bdb.la You don't need back_bdb as long as you're using back_hdb only. > +olcModuleLoad: ppolicy.la > +olcModuleLoad: unique.la These two modules are part of usersandgroups. I'd move them to usersandgroups/modules.ldif. [...] > +olcModuleLoad: refint.la I think this module should also be part of userandgroups. > === added file 'core/cosine-schema.ldif' > === added file 'core/inetorgperson-schema.ldif' > === added file 'core/misc-schema.ldif' [...] I would use .schema.ldif as the prefix instead of -schema.ldif: * cosine.schema.ldif * inet-org-person.schema.ldif > === added file 'core/monitor-acl.ldif' [...] > +dn: olcDatabase=monitor,cn=config As this define a new database how about creating a new module called monitor/ (at the same level as core/)? The following files would be part of it: * monitor/database.ldif * monitor/modules.ldif * monitor/acls.ldif One principle to follow in naming conventions in filenames. > === added file 'core/refint-overlay.ldif' > === added file 'core/unique-overlay.ldif' As suggested for schemas, I would use .overlay.ldif suffices. > --- core/unique-overlay.ldif 1970-01-01 00:00:00 +0000 > +++ core/unique-overlay.ldif 2010-06-17 16:58:16 +0000 > @@ -0,0 +1,11 @@ > +dn: olcOverlay=unique,olcDatabase={1}hdb,cn=config I'd try not to use indexes here as we're not sure the index number of the database. Computing the actual index should be left to the script that is actually responsible for loading these overlays in the correct database. > === added file 'doc/README.core' I would move the README to file to the core/ sub-directory in order to have the documentation colocated with the module. User and groups module ---------------------- I would suggest to use the same naming conventions (acls.ldif, modules.ldif, *.schema.ldif, *.overlay.ldif) as used in core/.