hi again,
I forgot one thing regarding: please avoid code
redundancy also with the cache-handling in svnurlcommand
(and everywhere else for that matter :) as well. E.g. lines such as:
auth = self.auth and self.auth.makecmdoptions() or None
self._lsnorevcache.delentry((self.dirpath().strpath, auth))
appear multiple times. It's probably best to rather use a helper like
self._norev_delentry(self.dirpath())
and have the helper method figure things out. Same is true
for the getorbuild where you might introduce a helper maybe as well.
I haven't checked in detail but maybe it's even possible
to have the helper disambiguate the cases of "norev" and "rev"
and use caches appropriately.
Btw, i have the subjective impression that caching
is not working properly - I remember the tests to run much faster.
thanks & cheers,
holger
On Sun, Feb 24, 2008 at 20:37 +0100, holger krekel wrote:
> Hey Guido,
>
> some first feedback ... could you refactor the tests such that
> e.g. changing the escaping of cmdline commands does not require
> lots of changes to test_auth.py? For example:
>
> def test_export(self):
> auth = SvnAuth('foo', 'bar')
> u = svnurl_no_svn('http://foo.bar/svn', auth=auth)
> target = py.path.local('/foo')
> u.export(target)
> assert u.commands[0].endswith('svn export "http://foo.bar/svn"; "/foo"
> '
> '--username="foo" --password="bar"')
>
> Apart from that this fails on windows, IMO it would be enough
> to test for the presence of the username/password combo
> and to do this from one place (and also the creation of "auth"
> from place). IOW: avoid redundancy and nitpicky output checking tests.
>
> For the slower functional test: could you maybe do some clever
> extra setup/teardown and re-use e.g. test_urlcommand.py's
> TestURLCommandPath tests? Some tests might not work, but then
> it's maybe better to rework those. If adding another base line
> for Path APIs i wouldn't like to have to write tests at several
> places for it. IOW: integrate tests some more with existing
> tests.
>
> hope this makes some sense, I am probably quite busy the
> next days.
>
> best,
>
> holger
>
>
>
>
> On Wed, Feb 20, 2008 at 22:43 +0100, Guido Wesdorp wrote:
> > Hi there!
> >
> > I've implemented auth support for py.path.svnurl now too, seems to work
> > nicely. Since it requires authentication for just about all its methods,
> > I decided (after some discussion with Holger on IRC and some playing
> > around) to not have an 'auth' argument to all of the methods. Instead
> > only the constructor groks an auth argument, path objects that are
> > generated by any of the path's methods 'inherit' it, and an
> > 'svnurl.auth' property is exposed to allow overriding it. It took a bit
> > of playing around to get it to work properly, but I think it does now,
> > please take a look if you're interested.
> >
> > If you like the way this works, I would (again ;) like to ask whether
> > I'm allowed to change svnwc so it works the same there - at least, both
> > for consistency's sake and because it's a bit cleaner imo - have an auth
> > property instead of having the argument to all methods that can connect
> > to the server would be nice...
> >
> > Please let me know what you think.
> >
> > Cheers,
> >
> > Guido
> > _______________________________________________
> > py-dev mailing list
> > py-dev@codespeak.net
> > http://codespeak.net/mailman/listinfo/py-dev
> >
>
> --
> Holger Krekel - freelance manager and programmer
> pylib py.test/greenlets/svn APIs: http://pylib.org
> PyPy Python/Compiler tool chain: http://codespeak.net/pypy
> merlinux collaborative contracting: http://merlinux.de
> _______________________________________________
> py-dev mailing list
> py-dev@codespeak.net
> http://codespeak.net/mailman/listinfo/py-dev
>
--
Holger Krekel - freelance manager and programmer
pylib py.test/greenlets/svn APIs: http://pylib.org
PyPy Python/Compiler tool chain: http://codespeak.net/pypy
merlinux collaborative contracting: http://merlinux.de
_______________________________________________
py-dev mailing list
py-dev@codespeak.net
http://codespeak.net/mailman/listinfo/py-dev
