Hi Phillip Kern! On Sun, Oct 03, 2010 at 10:25:35AM +0200, Philipp Kern wrote: > (I did review the git branches debian-experimental against the current unstable > version, which is the same as testing in all cases.) (Thats correct, no uploads to unstable since the freeze. I actually went ahead and uploaded to experimental, so reviewing either git or experimental is fine.) > > On Thu, Sep 16, 2010 at 07:18:27PM +0200, Andreas Henriksson wrote: > > * gssdp: > > > > http://git.debian.org/?p=collab-maint/gssdp.git;a=shortlog;h=refs/heads/debian-experimental > > > > http://git.debian.org/?p=collab-maint/gssdp.git;a=blob;f=NEWS;h=d222e10b7866db849ff41108c38c7cc87ae332c1;hb=refs/heads/upstream-experimental > > This is ok, as gssdp-socket-source.h isn't installed. > Thanks! > > * gupnp: > > > > http://git.debian.org/?p=collab-maint/gupnp.git;a=blob;f=NEWS;h=cb4e6291147df8f873934576db4cfb45e27e68ea;hb=refs/heads/upstream-experimental > > > > http://git.debian.org/?p=collab-maint/gupnp-av.git;a=shortlog;h=refs/heads/debian-experimental > > As there were no symbols files for the old one in unstable: are you sure there > are no disappearing symbols? (Maybe gupnp_context_manager_finalize...) If Upstream seems to have a very good clue when it comes to ABI. c.f. gupnp-av and the introduction of the _size64 function rather then fixing (and breaking ABI of) the _size function, for large file support on 32bit arches. That was done intentionally to avoid ABI breakage. gupnp_context_manager_finalize was static, so it shouldn't affect the ABI. (The good upstream ABI clue might be related to our fellow DD, Ross Burton, is (part of) upstream. :)) > people could stop reindent files, that would be helpful, too. gvalue-util.c > doesn't make me happy. Using strtoul instead of atoi... why?[1] While the page you refer to points out some flaws with strtoul, atoi really isn't any better. It's probably of questional "bug fix" value though, just like the reindentation... > > The other changes look fine. > Thanks! > > * gupnp-av: > > > > http://git.debian.org/?p=collab-maint/gupnp-av.git;a=shortlog;h=refs/heads/debian-experimental > > > > http://git.debian.org/?p=collab-maint/gupnp-av.git;a=blob;f=NEWS;h=966e33beb5a3102a54dc06d1f11f6f6efff0d9c3;hb=refs/heads/upstream-experimental > > Looks ok. > Thanks! > > * gupnp-dlna (no change from last request, but here are the links anyway): > > > > http://git.debian.org/?p=collab-maint/gupnp-dlna.git;a=shortlog;h=refs/heads/debian-experimental > > > > http://git.debian.org/?p=collab-maint/gupnp-dlna.git;a=blob;f=NEWS;h=9ba258e13be8457eedef6bc1d1516f1b514a13e8;hb=refs/heads/upstream-experimental > > There isn't a GNU Lesser General Public License version 2, right? That > replacement seems to be made of fail. Didn't review more, sorry. The license change (which probably is just a clarification of the original intent) is one of the reasons why I'd like to avoid shipping the old version. As for if a 2.0 version exists, most people still don't seem to tell Library or Lesser GPL apart, so my interpretation would be: http://www.gnu.org/licenses/old-licenses/lgpl-2.0.html I don't want you to waste time on reviewing this package. I think it's completely harmless to update it since it has no reverse dependencies in debian. If you're still not convinced, I might go ask for a removal instead of supporting the 0.2 version of gupnp-dlna through the squeeze lifetime. Newer versions is already a requirement of known users anyway. > > Kind regards, > Philipp Kern > > [1] http://synflood.at/blog/index.php?/archives/741-strtoul-considered-harmful.html I'll go ahead and upload everything to unstable now (and get back to you for the unblocks when ready). If you come to the conclusion after a full gupnp-dlna review that it's unsuitable, I'll go for a removal from squeeze for gupnp-dlna. Thanks again for reviewing! -- Andreas Henriksson
Attachment:
signature.asc
Description: Digital signature