[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Improve svn_checksum_t bindings in SWIG

From: Daniel Shahaf <danielsh_at_elego.de>
Date: Tue, 11 Dec 2012 10:47:42 +0200

Shivani Poddar wrote on Tue, Dec 11, 2012 at 11:51:19 +0530:
> On Tue, Dec 11, 2012 at 4:08 AM, Daniel Shahaf <danielsh_at_elego.de> wrote:
> > Shivani Poddar wrote on Tue, Dec 11, 2012 at 02:22:28 +0530:
> > > Index: subversion/bindings/swig/python/tests/checksum.py
> > > ===================================================================
> > > --- subversion/bindings/swig/python/tests/checksum.py (revision 1419694)
> > > +++ subversion/bindings/swig/python/tests/checksum.py (working copy)
> > > @@ -20,22 +20,25 @@
> > > #
> > > import unittest, setup_path
> > > import svn.core
> > > -
> > > +LENGTH = 32 or 40;
> >
> > This is wrong in two different ways:
> >
> > - "32 or 40" is a constant expression that evaluates to 32.
> >
> > - You hardcode two values, when you should be hardcoding neither of
> > them. (The bindings shouldn't need to change if the C library grows
> > a third svn_checksum_kind_t.)
> >
> > the symbolic constants in python are declared as this one. However in this
> test, since we are checking by only svn_checksum_md5 , we only need the
> length to be >= 32, i dont know why we would want to include 40 in the

I didn't ask you to include 40. And if didn't know why to include it,
why _did_ you include it?

> length here , since atleast in this test length should always be 32.
> so maybe
> LENGTH =
> svn.core.svn_checksum_to_cstring_display(svn_checksum_create(svn_checksum_md5))
> would have been a better thing to do
>
> > > + if(int(check_val) != 0):
> > > + raise
> >
> > This bare "raise" statement without arguments is itself an error.
> >
> > See for yourself:
> >
> > % python -c 'raise'
> > TypeError: exceptions must be old-style classes or derived from
> > BaseException, not NoneType
> >
> > This exception signifies a bug in your program. It has become
> > a RuntimeError in recent Pythons (and, frankly, could become
> > a compile-time error as well --- the compiler knows there's no except:
> > block surrounding this statement). It might work, but not because it's
> > correct.
> >
> > Yes it was working for me in the program, will check how i can fix this one

Please don't your own lines with a > character. Most people's clients
(including mine) will filter(lambda line: line.startswith('>'), lines)
the emails they display, so if you prepend a > to anything but the
quoted text you're replying to, it simply won't be seen or read by most
folks.
Received on 2012-12-11 09:48:36 CET

This is an archived mail posted to the Subversion Dev mailing list.