Samba4 Patch: newuseradv and newgroupadv scripts for net cmd utlity

[Andrew Bartlett]

On Tue, 2010-06-08 at 20:47 +0100, Lukasz Zalewski wrote:
> Hi Jelmer,
> 
> On 08/06/2010 19:39, Jelmer Vernooij wrote:
> > 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:
> Thx for all of the comments. I have attached modified patches
> >
> > * PEP8 - please use two empty lines between top-level elements (such as
> > classes).
> I hope this is fixed - I'm quite new to python
> >
> > * 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.
> newuser and newgroup functions corrected.
> >
> > * Any reason for using cmp() rather than == when comparing two strings ?
> > The latter is more readable and more commonly used.
> No reason, except old habbit of using strcmp i guess ;)
> >
> > Cheers,
> >
> > Jelmer
> Let me know if i need to make any more changes

There is one more thing that would be really useful.  If you look at
setup/tests/blackbox_newuser.sh, we have a test for the old version of
this script.  If you can extend it to cover all the new options you have
here, that would help ensure it keeps working.

Also make sure that the tree, with this patch in, passes 'make test'
just as much as without it. 

Thanks,

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org
Samba Developer, Cisco Inc.

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