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

Re: Accepted vlc 0.8.6-svn20061012.debian-3 (source i386 all)



On Wed, Jan 17, 2007 at 12:53:56PM +0100, Sam Hocevar wrote:
> On Wed, Jan 17, 2007, Steve Langasek wrote:

> > - 020_notify.diff includes a patch to prepend DATA_PATH to
> >   "/usr/share/pixmaps/vlc.png".  Why?  This isn't documented in the
> >   changelog, and doesn't make any sense to me anyway.

>    Yes, I messed up that one. Will fix.

> > - patch-documentation-0.8.6debian-0.8.6a.diff contains:
> >  [..]
> >   How is this a documentation fix?

>    It's not. I said on IRC last week that I would remove that chunk.
> Either I lied or I forgot about it.

> > - 000_bootstrap.diff seems to be enabling maintainer mode by default.  Why
> >   is this a) appropriate, and b) necessary?  Are you certain that there are
> >   no possibilities of timestamp skew here that would cause a FTBFS on
> >   autobuilders when autofoo are not installed?  (The answer here appears to
> >   depend on the order in which the patches are applied, which I haven't
> >   analyzed.)

>    No, it actually disables maintainer mode by default, to avoid the
> problem you mention.

Oh, I see; I overlooked that the change was to /comment out/ an upstream
setting that made maintainer mode the default.

> > - patch-sanitise-javascript-0.8.6debian-0.8.6a.diff: why is it /wrong/ to
> >   escape \ characters here?

>    Upstream tells me it's not needed, I'll remove it.

> >   this reverses the initialization of the .Cr and .Cb elements, and in fact,
> >   disagrees with an immediately preceeding section of the same patch.  Can
> >   you explain?  (It does agree with an immediately /following/ section of
> >   the patch, so perhaps it's the first section that's wrong?)

>    It's the first chunk of the patch that's wrong. I can fix it, leave
> it like that, or remove it, since it only has a visual cosmetic impact
> and the important part of the patch is the last chunk (uninitialised
> i_original_* variables) and I just kept the full patch to avoid later
> conflicts if patches need to be applied.

Ok, I'm fine with this either way then, so whichever you prefer.

> > The debdiff also includes changes specific to architectures which aren't
> > releasing with etch, and are apparently not planning to do their own
> > rebuilds of etch, and even includes added patches from unstable that have
> > only been disabled in the patch sequence file.  This makes for a very
> > time-consuming diff to review, and certainly leaves me less confident that I
> > haven't overlooked any regressions.  I would be much more comfortable
> > letting this straight into testing if this weren't the case.

>    I'm sorry about that. My thought was that even if the release team
> does not see these patches as relevant for testing, users or porters
> may benefit from the patches being at least present in the sources. (I
> also believe that unapplied patches for non-released architectures are
> probably at least partly accountable for the decay of some of those
> architectures).

Well, AFAIK none of those non-release archs are planning to base anything
off of etch, just sid for the time being; and you can help those archs by
applying patches to unstable without applying them to t-p-u -- and it's
fairly common that patches for bsd/hurd break support for the release archs.

Anyway, I would still be a little more comfortable if the changes to
debian/rules weren't included in the diff, but it's not a big enough deal
that I would refuse to accept the package.  So if you can make the three
other changes discussed above, I'll gladly let vlc into testing.

Thanks,
-- 
Steve Langasek                   Give me a lever long enough and a Free OS
Debian Developer                   to set it on, and I can move the world.
vorlon@debian.org                                   http://www.debian.org/



Reply to: