Trying again at using the python samba_kcc
asn at samba.org
Thu Apr 23 00:33:05 MDT 2015
On Thursday 23 April 2015 17:47:27 Andrew Bartlett wrote:
> On Tue, 2015-04-21 at 14:15 +0200, Andreas Schneider wrote:
> > On Friday 17 April 2015 21:44:21 Douglas Bagnall wrote:
> > > > 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.
> > I know it is a work in progress branch but there are a lot of commits
> > which
> > should be squashed into the patches where the code has been introduced.
> We have, as much as has been practical now done that. The code now runs
> in each commit (except in the rename/split out work at the end, that is
> still WIP). Remember that in this case, with the feature disabled by
> default in smb.conf, there is also no risk of lost bisect-ability.
> Given the complexity, recording these incremental development steps on
> the operating script is an important and helpful record in the aid of
> future development in this complex area, as it highlights the unexpected
> and/or undocumented aspects in small commits, and ensures these are not
> lost in the major rewrite.
> An updated branch is at
> tersite-19 git://git.catalyst.net.nz/samba.git kcc-intersite-19
Wouldn't it make a review more easier if the commit which introduces new code
already have correct documentation?
This looks like a mistake when the code was introduced. Shouldn't it be
already correct in the code which introduces the code so that I do not get
errors cause of the wrong type bing used?
Is this an attempt or a fix? If it is fixed in a later commit both should
probably be squashed and the commit message should indicate what it fixes.
Andreas Schneider GPG-ID: CC014E3D
Samba Team asn at samba.org
More information about the samba-technical