[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies



On Wed, Jan 01, 2014 at 12:30:16PM +0100, Michael Schaller wrote:
> A few comments on the patches:

Thanks for your review and the points you raised.

> 1) tests/test_pep8.py
> Can you document in a comment or docstring why you ignore certain
> issues and what these issues are? This should help readers to better
> understand what the test does and more importantly what it ignores.
> Furthermore do you plan to reduce the number of ignores in the
> future? If so you should add a TODO for yourself.
>
> 2) tests/test_pep8.py
> Can you use instead of 'self.assertEqual(res, 0)' a better fail
> message like you did in 'tests/test_pyflakes.py'?
> 
> 3) tests/test_pyflakes.py
> You don't need the 'from __future__ import print_function' import
> with Python 3.

Those are fixed in my git branch now, thanks!
 
> 4) debian/control (nitpick)
> I prefer to sort dependencies alphabetically.

I have no opinion about this :) I don't really mind either way.

> 5) .travis.yml
> I would really like to see a rather lengthy comment in the beginning
> of the YAML file to explain what purpose it serves and what it does.
> I also think you should give a link here and that you should explain
> what should happen after Trusty. Maybe it would be even better to
> use SID instead of Trusty.
>
> 6) .travis.yml
> Why do you run './debian/rules binary' and not 'dpkg-buildpackage'?
> 
> 7) .travis.yml
> Why not also ensure here that Lintian and if possible piuparts have
> no issues with the newly built package?

Good points, I added a explaination to the top now that hopefully
addresses the points.

I renamed the added sources.list entry as its misleading. In my branch
its using "distro-info --stable" to get test latest python3.3 and
apt/libapt. Using sid here is probably a bit too fragile, the chroots
of the travis-ci.org are ubuntu based. But I would certainly love to
see a travis-ci instance that is debian based.

I changed the "debian/rules binary" in my branch now to be
"./debian/rules build-arch". This will build and run the tests. We
can't use dpkg-buildpackage currently as its using fakeroot and the
tests (iirc in the test_auth.py code) also use fakeroot and nesting
is not possible AFAIK. But I do like the idea of building the deb and
doing additional checks like lintian/piuparts.

Cheers and happy new year!
 Michael


Reply to: