Re: new set of changes
On Sat, Sep 06, 2008 at 09:02:52PM -0500, Raphael Geissert wrote:
> Frank Lichtenheld wrote:
> > 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.
Sure.
> >
> >> + 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 :-/
Because I prefer to need 5 milliseconds to confirm that this is correct
code instead of 50. Yeah, I'm petty...
> >
> >> + 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.
Again, because the shorter code takes longer to read, if you know what I
mean. And "use English" would be only slightly longer, but much easier
to read in my opinion. YMMV.
> >
> >> + 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.
Indeed, I was confused.
Gruesse,
--
Frank Lichtenheld <djpig@debian.org>
www: http://www.djpig.de/
Reply to: