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: