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

Re: Bug#596247: future unblock: libtime-progress-perl/1.7-1



Hi Adam

Thanks for your reply and time!

On Thu, Sep 23, 2010 at 08:03:08PM +0100, Adam D. Barratt wrote:
> On Fri, 2010-09-10 at 14:10 +0200, Salvatore Bonaccorso wrote:
> > So upstream fixed already the small typo, and released the new
> > version. So the debdiff is slightly shorter, it is attached.
> 
> The subject suggested this was "could I upload this"; I see it's been
> uploaded already.

I'm sorry if this caused more troube as it was more intended to be a
'wishlist'. 

> +  * Convert to '3.0 (quilt)' source package format.
> 
> That's not really an appropriate change to be making during a freeze...
> 
> +  * debian/rules: Simplify to a three line rules file.
> 
> ... although imho it's better than that.

I agree that these are 'major' packaging changes. I hope I can convice
that they are not harmfull ;-). I introduced the change in the source
package format in the 1.5 -> 1.6 step, as libtime-progress-perl had no
patch before, then added the fix-spelling error. This way, reducing
the debian/rules and converting the source package format, we did not
need to add quilt to Build-Depends, nor bump Build-Depends on
debhelper (>= 7.0.8). The packaging of Perl modules are 'smooth' in
general, so even the packaging introduced this major changes, it does
not 'break'. 

> +helpers -- return elapsed/estimate seconds.
> 
> s/estimate/&d/

I assume the point here from upstream author was more, to 'align' the
documentation to the really implemented method name. See in the
debdiff:

---(snip)---------------------------------------------------------------
 =item elapsed
 
-=item estimated
+=item estimate
+
+helpers -- return elapsed/estimate seconds.
------------------------------------------------------------------------

and indeed in the code, they are implemented with that name:

---(snip)---------------------------------------------------------------
 sub elapsed
-{ my $self = shift; return $self->report("%l"); }
+{ my $self = shift; return $self->report("%l",@_); }
 
 sub elapsed_str
-{ my $self = shift; return $self->report("elapsed time is %L min.\n"); }
+{ my $self = shift; return $self->report("elapsed time is %L min.\n",@_); }
 
 sub estimate
-{ my $self = shift; return $self->report("%e"); }
+{ my $self = shift; return $self->report("%e",@_); }
 
 sub estimate_str
-{ my $self = shift; return $self->report("remaining time is %E min.\n"); }
+{ my $self = shift; return $self->report("remaining time is %E min.\n",@_); }
------------------------------------------------------------------------

My assumption here is that upstream did not change the implemented
subroutine name, to not break existing code using it, but adjust the
documented name to be in line with that.

See [1] for details about this.

 [1] https://rt.cpan.org/Ticket/Display.html?id=58127

I'm not a native english speaker, but from this view estimated would
be the right term. I feel like for squeeze at least it should be that
adding the documentation change, so that it is consistent to the names
implemented.

One further point is that on the other side, the methods as they where
implemented, always returned 0, so the above diff, fixed that, and the
4 methods should work now as they where intended, see the above RT
ticket on  CPAN.

> +all helpers need one argument--current item.
> 
> s/--/ -- / ?

Ok, I will forward this too to upstream.

Bests
Salvatore

Attachment: signature.asc
Description: Digital signature


Reply to: