[webkit-reviews] review denied: [Bug 28907] [Qt] The --strict switch of run-webkit-tests does not work : [Attachment 39149] Refactor --strict switch to --ignore-metrics

[bugzilla-daemon at webkit.org]

Tor Arne Vestbø <vestbo at webkit.org> has denied Andras Becsi
<becsi.andras at stud.u-szeged.hu>'s request for review:
Bug 28907: [Qt] The --strict switch of run-webkit-tests does not work
https://bugs.webkit.org/show_bug.cgi?id=28907

Attachment 39149: Refactor --strict switch to --ignore-metrics
https://bugs.webkit.org/attachment.cgi?id=39149&action=review

------- Additional Comments from Tor Arne Vestbø <vestbo at webkit.org>
Nice. A few finishing touches:

> +my $simpleTag = "simple";

This is no longer needed.

> +    'ignore-metrics' => \$ignoreMetrics,

Should be postixed with a ! since it's a boolean flag, this will make
GetOptions support --no-ignore-metrics, etc

>      'strip-editing-callbacks!' => \$stripEditingCallbacks,

Like this one

> +sub simplify($$)

Suggest renaming to stripMetrics()

> +    my $simpleExpected = $expected;
> +    my $simpleActual = $actual;

No need to assign these, you already have it in $actual and $expected.

> +    $simpleActual = $actual;

And def. no need to re-assign a second time ;)

> +    $simpleActual =~ s/at \(-?[0-9]+,-?[0-9]+\) *//g;

Just use $actual here, it's passed by value

> +    $simpleExpected =~ s/at \(-?[0-9]+,-?[0-9]+\) *//g;

Same, just use $expected directly