On 5/10/06, Jelmer Vernooij <jelmer@samba.org> wrote:
> On Wed, 2006-05-10 at 15:10 -0700, Garrett Rooney wrote:
> > On 5/10/06, Jelmer Vernooij <jelmer@samba.org> wrote:
> > > The attached patch fixes the python bindings for svn.ra.get_dir2() and
> > > svn.ra.get_dir(). Previously these functions would only return the list
> > > of properties on a directory; it now returns a tupel with a list of
> > > dirents, revnum that was actually fetched and the list of properties.
> Ok, second try :-)
>
> > 1) Your log messages don't really follow our guidelines. You have
> > good summaries in your email, but they need to make it into the log
> > message as well. Also, if you're modifying a file that has well
> > defined functions, you need to specify which functions are being
> > modified as part of the message. See how I modified your last patch's
> > log for details.
>
> Does this look better? I'm having trouble mapping the examples on the
> website to SWIG.
It's reasonable, although I'd put a sentence or two summary at the
top, something like "Make svn_ra_get_dir2 work in the python bindings
by adding some more typemaps". Gives people a quick idea what the
change does without having to go through all the details. I'll add
that this time though, don't worry about it.
> Changes:
>
> [[[
> * subversion/bindings/swig/svn_ra.i
> subversion/bindings/swig/include/apr.swg
> (apr_hash_t **DIRENTHASH): Add typemap
> (apr_hash_t **PROPHASH): Allow use in tuples
>
> * subversion/bindings/swig/python/tests/ra.py
> (test_get_dir2): Add test for svn.ra.get_dir2()
>
> ]]]
>
> > 2) We don't use tabs in our code, and your indenting should in general
> > try and match the style of the code around it. This includes trying
> > to break lines before they hit 80 columns.
> Should be fixed now.
Cool.
> >
> > Other than that keep up the good work. If you come up with some tests
> > for this one so I can see that it works I'll be happy to apply,
> > otherwise I'm sure someone with more swig-fu will come along
> > eventually.
> I've added some simple tests for get_dir2 in ra.py in the updated
> patch.
Great, I'll take a look.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu May 11 19:19:48 2006