Bug#717995: [etienne.millon@gmail.com: Re: RFS: feedparser/5.1.2-2]
(Attaching Etienne's comments with permission, in case this is useful to
anyone else.)
----- Forwarded message from Etienne Millon <etienne.millon@gmail.com> -----
Date: Mon, 29 Jul 2013 14:45:26 +0200
From: Etienne Millon <etienne.millon@gmail.com>
To: Adam Sampson <ats@offog.org>
Subject: Re: RFS: feedparser/5.1.2-2
> Speaking of adoption and packages that use feedparser...
>
> If you've got a minute, is there any chance you could have a look
> over the rawdog 2.16-1 package I've just uploaded to mentors?
>
> http://mentors.debian.net/package/rawdog
>
> I'm the upstream maintainer of rawdog, and have ITA-d the rawdog
> Debian package since it's currently orphaned. Since one of the
> changes in the new upstream version is to make it depend on
> feedparser rather than bundling it, it'd be useful to check I've not
> done anything stupid from the point of view of the python-feedparser
> maintainer. ;-)
Hi,
Thanks for taking care of rawdog. Incidentally, getting rid of the
embedded feedparser copy was the first thing I did with rss2email
too :)
It seems to me that you did a good job at implementing the "best
practices" for rawdog: 3.0 (quilt) format (even if it's not used yet),
DEP-5 copyright file, modern dh, watch file...
Here is my review of your changes. As noted in the header it's a
debdiff to the version currently in unstable.
> --- rawdog-2.13.dfsg.1/debian/changelog 2013-07-29 13:54:49.000000000 +0200
> +++ rawdog-2.16/debian/changelog 2013-07-28 23:44:46.000000000 +0200
> @@ -1,3 +1,23 @@
> + * Update package to meet Debian standards version 3.9.4.
I think that you should say what you changed, or just put "(no changes
needed)" in case you went through the upgrading checklist and found no
problems.
> --- rawdog-2.13.dfsg.1/debian/control 2013-07-29 13:54:49.000000000 +0200
> +++ rawdog-2.16/debian/control 2013-07-28 23:43:30.000000000 +0200
> @@ -1,22 +1,23 @@
> -Maintainer: Decklin Foster <decklin@red-bean.com>
> -Build-Depends: debhelper (>= 5.0.37.1), python, python-dev
> -Build-Depends-Indep: python-support (>= 0.5.3)
> -Standards-Version: 3.8.0
> +Maintainer: Adam Sampson <ats@offog.org>
> +Build-Depends: debhelper (>= 9), python (>= 2.6.6-3~),
> + python-feedparser (>= 5.1.2), python-tidylib
The python dependencies may actually stay Build-Depends-Indep, but I
don't think that the distinction is actually used. So, do as you wish.
See this page if you're interested in the rationale:
http://wiki.debian.org/Build-Depends-Indep
> --- rawdog-2.13.dfsg.1/debian/copyright 2013-07-29 13:54:49.000000000 +0200
> +++ rawdog-2.16/debian/copyright 2013-07-22 22:45:03.000000000 +0200
> +Files: *
> [...]
> +Files: rawdoglib/feedscanner.py
I believe that it's missing the copyright info relative to debian/*
files. It's common to license it with the same license as the upstream
code, but of course it's up to the copyright holders. The code there
is so small there that it's barely copyrightable though...
> --- rawdog-2.13.dfsg.1/debian/rules 2013-07-29 13:54:49.000000000 +0200
> +++ rawdog-2.16/debian/rules 2013-07-28 23:26:26.000000000 +0200
> +ifeq ($(filter nocheck,$(DEB_BUILD_OPTIONS)),)
> +override_dh_auto_test:
> + ./test-rawdog
> + rm -fr testauto
> +endif
I'm a bit surprised that you have to test for nocheck here, but as
Jakub noted the test is done in dh_auto_test, not dh, so it's
necessary (but strange).
I also had a quicker look at the upstream changes:
> diff -Nru rawdog-2.13.dfsg.1/PKG-INFO rawdog-2.16/PKG-INFO
> --- rawdog-2.13.dfsg.1/PKG-INFO 2010-10-15 23:38:53.000000000 +0200
> +++ rawdog-2.16/PKG-INFO 2013-07-15 23:45:30.000000000 +0200
> -License: GNU GPL v2 or later
> +License: UNKNOWN
I don't think that this is wanted :) That's probably because you
removed the "license=" key in setup.py.
> +Replace feedfinder, which has unfixable unclear licensing, with the
> +module that Decklin Foster wrote for his Debian package of rawdog
> +(specifically rawdog_2.13.dfsg.1-1). I've renamed it to "feedscanner",
> +on the grounds that it may be useful to other projects as well in the
> +future.
Great! I didn't know about feedfinder and the related license issue.
Thanks for maintaining feedscanner too. I might push support for this
useful library to upstream rss2email. Would you be interested in
packaging it separately? (not necessarily now of course).
The package builds dine, but the test suite does not seem to run on my
machine:
> >>> Testing filename not found
> Feed: missing.rss
> Error while fetching feed:
> <urlopen error [Errno 2] Aucun fichier ou dossier de ce type:
> 'missing.rss'>
Setting LANG=C works, you should add it in the runf() function.
There's also this one, but I didn't debug it:
> >>> Testing item dates
> [...]
> Test failed: expected testauto/output.html to contain
> 'HEADING-01-01-18:00'
> make[1]: *** [override_dh_auto_test] Error 1
Other than this test suite problems, the package seems fine with me.
Thanks again for your work and hope to see you in unstable soon!
--
Etienne Millon
----- End forwarded message -----
Reply to: