Comment 10 for bug 180624

Revision history for this message
Loïc Minier (lool) wrote : Re: cheese new upstream version 0.3.0

Hi Steve,

Good progress in this updated interdiff; here's an updated review:

I) I see you documented the removal of patches as "Removed debian/patches fixed in this release"; however, in my reading, the functionality of each patch is still missing from the new upstream release; could you explain why you think each one can be dropped with more details? For example, if you see what the fix_desktop patch fixed, perhaps you can explain why you think it can be completely dropped? I think the .desktop file still needs fixing in the new upstream release, as only part of the fix was merged.
Concerning the hildon patch, could you check the debian/changelog and tell me whether you understand what it is about? If you can't find out, I shall explain it and we will document it in the patch.

II) In your previous version, you dropped debian/docs, and in the new one you don't touch it at all; but upstream used to ship a TODO file and doesn't do so anymore -- does the package build if the TODO is listed in debian/docs but not present?

III) Could you please revert or explain the reasons why you changed in debian/rules:
--include /usr/share/gnome-pkg-tools/1/rules/gnome-get-source.mk
+include /usr/share/gnome-pkg-tools/1/rules/gnome-get-source.mk
and in debian/changelog:
 cheese (0.2.4-0ubuntu3) hardy; urgency=low
- * Watch file added (LP: #180380)
+ * Watch file added (closes LP #180380)

IV) You changed the watch file, but you didn't mention it in your new 0.3.0-0ubuntu1 changelog entry

V) a) I see you fixed most of the build-deps, that's nice! However, could you please recheck the required version for libglade2-dev, libgnomeui-dev?

b) Also, you added a build-dependency on libgconf2-4, I think you meant libgconf2-dev.

c) The configure.ac checks as follows:
  gstreamer-0.10 >= $GSTREAMER_REQUIRED \
  gstreamer-plugins-base-0.10 >= $GSTREAMER_REQUIRED \
please update the libgstreamer-plugins-base0.10-dev accordingly.

VI) Very minor sugar spacing issues:
- still adding trailing space to the debian/rules shebang -- see interdiff
- "libgnomeui-dev (>=2.0)" => note missing space after >=
- Homepage control field misses a trailing /
- You use http://ftp.gnome.org in the watch file but http://www.gnome.org/ in the copyright; I'd suggest using ftp.g.o everywhere