On Sat, May 31, 2008 at 05:54:31AM -0700, James Keenan via RT wrote:
> Please review the patch attached, which implements your suggestion.
The patch looks good to me; I just have a couple of questions
and/or observations:
> Index: lib/Parrot/Harness/Options.pm
> ===================================================================
> --- lib/Parrot/Harness/Options.pm (.../trunk) (revision 27904)
> +++ lib/Parrot/Harness/Options.pm (.../branches/codetests) (revision 27965)
> ...
> @@ -87,6 +90,7 @@
> --core-tests
> --runcore-tests
> --html
> + --code
> EOF
Since the other tests are specified using "-tests" in the option
name, perhaps we should have "--code-tests" instead of just "--code"?
(c.f. --core-tests and --runcore-tests).
> Index: config/gen/makefiles/root.in
> ===================================================================
> --- config/gen/makefiles/root.in (.../trunk) (revision 27904)
> +++ config/gen/makefiles/root.in (.../branches/codetests)
> (revision 27965)
> @@ -1513,6 +1514,9 @@
> perl_tests :
> $(PERL) t/harness $(PERL_TEST_FILES)
>
> +codetest :
> + $(PERL) t/run_code_tests
> +
Shouldn't this read...?
codetest :
$(PERL) t/harness $(EXTRA_TEST_ARGS) --code-tests
After the patch takes the above into account, I think it would
be okay to apply it.
Thanks again!
Pm
