Samba4 Patch: newuseradv and newgroupadv scripts for net cmd utlity

Jelmer Vernooij jelmer at samba.org
Tue Jun 8 12:39:34 MDT 2010


Hi Lukasz,

On Mon, 2010-06-07 at 17:29 +0100, Lukasz Zalewski wrote:
> Attached is a new patch which is an extension to the previous newuser 
> functionality. It also adds the set of group commands (operating on the ldb)
> 
> Now, new user is created with CN='Firstname Initials. Lastname',<> if 
> any of them exists and --use-username-as-cn is not set (this is the AD 
> way) otherwise CN=username,<> is used
> If --use-username-as-cn is defined then the user is created with 
> CN=username,<> even if any of (Firstname, Initials,Lastname) exist
> 
> I also took liberty to add group set of subcommands, i.e.
> /usr/local/samba/bin/net group
> Available subcommands:
> 	addmembers
> 	add
> 	removemembers
> 	delete
> 
> These only operate on ldb at the moment.
Thanks! This looks very nice overall. Some minor comments:

* PEP8 - please use two empty lines between top-level elements (such as
classes).

* When dealing with transactions it's usually better to delay starting
the transaction as long as possible (where it makes sense, of course).
This makes the code more readable (less indentation). E.g. at the bottom
of your patch you create a ldbmessage after starting a transaction
rather than just for the self.add call. There's no need to go through a
lot of trouble to do this but if it's easy it's preferable. 

* Any reason for using cmp() rather than == when comparing two strings ?
The latter is more readable and more commonly used.

Cheers,

Jelmer
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20100608/c1128be0/attachment.pgp>


More information about the samba-technical mailing list