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

Re: Review of controllib-removal branch requested



Hi,

On Wed, 16 Jan 2008, Guillem Jover wrote:
> On Tue, 2008-01-01 at 21:15:02 +0100, Raphael Hertzog wrote:
> > I just finished the necessary work to get rid off controllib.pl. I pushed
> > it in a new branch controllib-removal:
> > http://git.debian.org/?p=dpkg/dpkg.git;a=shortlog;h=controllib-removal
> 
> > I welcome some review on the changes (it amounts to 35 changesets of which
> > half comes from djpig's parsechangelog branch).
> 
> The Format substvar is not set anymore.

Indeed, that was on purpose. It wasn't used by the code as a substitution
but only like a general purpose variable (which I really created as is in
dpkg-genchanges) and I have never seen any package use it. And it didn't
make sense to store the version of the format of the changes file in all
substvar objects that are used to make substitutions also in binary
packages and source packages.

What would you suggest then ? Shall I add $substvar->set("Format",
$changes_format) in dpkg-genchanges ?

> In dpkg-source.pl, $varlistfile should not be initialized, this is a
> functional change. I've actually been considering for some time
> removing the substvar support from dpkg-source.

It is a functional change indeed but it only brings it in line with the
documentation, cf dpkg-source.1:
       -Tsubstvarsfile
              Read substitution variables in substvarsfile; the default is debian/substvars.

As you say, if it's not initialised, we might as well drop the substvars
support in dpkg-source.

> Also this is minor and it's done already, but while you were at it you
> could have fixed indentation and missing spaces for the touched lines.

I usually do that except when it's just a big copy/paste or a simple
reindentation (because I removed an enclosing if, or something like that).
I'll try to keep that in mind for the future.

-- 
Raphaël Hertzog

Le best-seller français mis à jour pour Debian Etch :
http://www.ouaza.com/livre/admin-debian/


Reply to: