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

Re: Restructuring check scripts



Russ Allbery wrote:

> Raphael Geissert writes:
>> Russ Allbery wrote:
> 
>>> Why did you move the arguments to the run method into object data?  I
>>> think they're better left as arguments that are passed to every check
>>> function.  It's one fewer level of indirection.
> 
>> I though setting up once and calling many was better. I didn't like the
>> amount of changes needed, but I decided to go on and see what you'd say.
> 
> I suspect it's a wash from a performance standpoint, and I think it's a
> bit clearer to keep passing in the arguments to the run script.

Ok. I've already done that on my local branch.

>>> However, I don't think the parent class here is useful; I would just
>>> move the run code from below directly into Checker.pm rather than
>>> creating the parent object.
> 
>> You mean blessing the object directly in run()?
> 
> More that you can get rid of the object completely and just do the direct
> sub calls that the run method was doing, I think (although it's been a
> while since the thread, and I should have responded right away so that I
> remember what I was thinking).

Ah, you mean to treat them like static methods?

> 
>>>> +sub run {
>>>> +    my $self = shift;
>>>> +    my $class = ref $self;
>>>> +    my @table;
>>>> +
>>>> +    eval("\@table = keys %". $class ."::;");
>>> 
>>> Unnecessary eval
> 
>> I prefer the cleaner approach without disabling strict references; but am
>> going to do it without the eval only because of the performance penalty.
> 
> Well, usually using string eval is considered much more problematic than
> disabling strict references.  Disabling strict references bypasses a bit
> less in the way of checking.  (Plus, yeah, performance.)

Do you know how Test::Class works?
I don't fully understand how the 'sub name : Something' part is implemented.
A similar approach could be used in Lintian, to make the code even more
flexible and easier to understand.

> 
>>> sort @table.  We want to run checks in a predictable order.
> 
>> Why? Anything relying on a predictable order is likely to be bogus.
>> That's why for example the tests use sort(1) on lintian's output.
> 
> Consider, for instance, checks for alternative output formats, where
> sorting the Lintian output can make the tags file in the Lintian test
> suite unreadable.  (Right now, by chance all of our output formats use one
> line per tag, but I expect that to change eventually for at least the XML
> output method.)

True.

> 
> Also, whenever there are problems, it's a lot easier to track them down if
> the tests are run in the same order for us as they are for the person
> reporting a problem.
> 

Of course :) (unless there's a --predictable option ;-)

Cheers,
-- 
Raphael Geissert - Debian Developer
www.debian.org - get.debian.net



Reply to: