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

Re: new set of changes



Frank Lichtenheld wrote:

> On Wed, Jul 30, 2008 at 08:45:08PM -0500, Raphael Geissert wrote:
>> Hi all,
>> 
>> Attached is a patch making the following changes:
>> * correctly parse line continuations
>> * avoid trying to perform any extra check on version 1 watch files
>> * know about versionmangle when checking if there is any mangling because
>> of $repack
>> * add line information to some of the tags
>> * attempt to detect when watch file tries to access a sf.net mirror
>> directly (projectfoo.sf.net/some_file-(.+) and similar of course don't
>> cause false positives)
>> * debian-watch-file-specifies-wrong-upstream-version
>> * debian-watch-file-specifies-old-upstream-version
> 
> I would prefer if you could split that up in separate patches for the
> separate issues. Also if you could submit it in git patch format or
> offer a public git repository to pull from that would make applying
> them easier.

Is a single mbox fine? Attached is one, but doesn't contain the testsuite
entries.

> 
>> +    while (my ($dversion, $lines) = each %dversions) {
>> +        next if (!defined($dversion) || $dversion eq 'debian');
> 
> I would prefer either using "or" here or using parantheses around the eq
> term, just to make it clear that this is correct.

Why? I can read it perfectly :-/

> 
>> +        local $" = ', ';
> 
> Hmm, I would prefer not to use too many of Perl's special variables
> (IMHO everything other than $_, $/ and $.). I would recommend either us
> ng explicit joins or "use English".

Again, why? I mean, they are nice features.

> 
>> +        if (!$info->native &&
>> exists($changelog_versions{'orig'}{$dversion})) {
>> +            tag 'debian-watch-file-specifies-wrong-upstream-version',
>> "$dversion: @{$lines}";
>> +            next;
>> +        }
>> +        if (exists($changelog_versions{'mangled'}{$dversion})
>> +            && $changelog_versions{'mangled'}{$dversion} != 1) {
>> +            tag 'debian-watch-file-specifies-old-upstream-version',
>> "$dversion: @{$lines}";
>> +            next;
>> +        }
>> +    }
>> +}
> 
> I believe we will need to add changelog-file to Needs-Info for that to
> reliably work.

Hmm, isn't changelog-file specific to binary packages? watch-file operates
on source packages.

> 
> Just out of curiousity: how many packages in the archive are actually
> using this feature (and do not use "debian")?

SELECT COUNT(name) FROM pkgs WHERE watch LIKE '%uupdate%' AND watch NOT
LIKE '%debian%';
 count
-------
     7

I tried to run other queries but there are many FP's, but I know there are
more package using that feature.

What I did find was this "thing":

mksh | version=3
opts="uversionmangle=s/^([0-9]+)$/$1.1/;s/^([0-9]+)b$/$1.2/;s/^([0-9]+)c$/$1.3/;s/^([0-9]+)d$/$1.4/;s/^([0-9]+)e
$/$1.5/;s/^([0-9]+)f$/$1.6/;s/^([0-9]+)g$/$1.7/;s/^([0-9]+)h$/$1.8/;s/^([0-9]+)i$/$1.9/"
\
http://www.mirbsd.org/MirOS/dist/mir/mksh/ mksh-R([0-9]+[b-i]?)\.cpio\.gz

> 
>> -http://qa.debian.org/watch/sf.php?project=foo scripts\.([\d.]+)\.tar\.gz
>> +http://qa.debian.org/watch/sf.php?project=foo scripts\.([\d.]+)\.tar\.gz
>> debian uupdate
>>  
>> +version=3
>> +http://ftp.sf.net/foo/foo_bar(.+)\.Z 5 uupdate
>> \ No newline at end of file
> 
> Is that missing newline intentional?

Nope, but I didn't change it either :)

> 
> Gruesse,

Cheers,
-- 
Atomo64 - Raphael

Please avoid sending me Word, PowerPoint or Excel attachments.
See http://www.gnu.org/philosophy/no-word-attachments.html

Attachment: lintian_watch-file-checks.mbox
Description: application/mbox


Reply to: