Trying again at using the python samba_kcc

Douglas Bagnall douglas.bagnall at catalyst.net.nz
Fri Apr 17 03:44:21 MDT 2015


hi Jelmer,

On 17/04/15 01:06, Jelmer Vernooij wrote:
> Hi Douglas,
> 
> On Thu, Apr 16, 2015 at 04:20:06PM +1200, Douglas Bagnall wrote:
>>>>> http://git.catalyst.net.nz/gitweb?p=samba.git;a=shortlog;h=refs/head/kcc-intersite-8
>>>>> [...]
>>>> Can you look over this updated branch:
>>>> http://git.catalyst.net.nz/gitweb?p=samba.git;a=shortlog;h=refs/head/kcc-intersite-10
>>
>>> I'll have a look at the changes (delta between master and this branch) when I
>>> can and will follow up.
>>
>> As you can see, we have a monotonically increasing series of branches,
>> due to some ping-ponging between developers. Our current favorite is
>>
>> http://git.catalyst.net.nz/gitweb?p=samba.git;a=shortlog;h=refs/heads/kcc-intersite-14
>>
>> so please look at that if and when you can.
> 
> I've only had a quick look and read through the first set of changed files.
> Some superficial comments:
> 
> * please follow the style guidelines:
>  - no lines over 80 characters
>  - PEP8: spaces around equals signs
>  - PEP8 : empty line before first method in class definition, and two empty lines between
>    top level objects like classes
>  - PEP8: "not foo" rather than "foo == False"
>  - PEP8: Please use single-line summaries in docstrings, and document parameters

I believe we have addressed most of these style issue later in the
patch series (though not the docstrings -- thanks for raising that).
Nevertheless I am still not really pleased by the overall style.

> * some unit tests for kcc would be nice, at least for some functions

Yes!

> * samba.ldif_utils seems misnamed - it seems like it's more samba.samdb.ldif_utils?

Right. I'll look at that next week.

thanks.

Douglas


More information about the samba-technical mailing list