[cairo] [PATCH v3 0/7] test: Don't ignore test output files when written to the wrong directory

[Uli Schlachter]

Hi,

I started merging this and got up to patch 6/7, then stopped. Some comments below.

On 15.08.2013 20:08, Bryce W. Harrington wrote:
[...]
> Bryce Harrington (7):
>   test: Don't ignore test output files left in test directory

>   test: Fix several tests to place output files in the output directory
>   test: Use CAIRO_TEST_OUTPUT_DIR for name of the output directory
>   test: concat CAIRO_TEST_OUTPUT_DIR at point of use instead of
>     BASENAME

The first patch adds a hardcoded "output/" to some file names, the second one
changes that into a define and the third one completely undoes the first patch
and moves the addition of the "output/"-prefix from the define into the code.

I don't see why this is spread over three different commits. I would just squash
them together into a single one (which then would be about as large as each of
the individual commits, so splitting this up doesn't produce smaller patches).

It just seems like these are fixup commits which correct errors from the earlier
commit.

>   test: Make cairo_test_mkdir() usable throughout tests

Why does this copy&paste _cairo_test_mkdir() to cairo_test_mkdir()? Wouldn't it
be enough to rename _cairo_test_mkdir() and make it non-static (and adding it to
cairo-test.h)?

>   test: Ensure output dirs exist, falling back to current dir if needed

I don't know what I should say. I read the description, then looked at the diff
and afterward had to read the description again. This is the point were I
stopped committing the patches, because this makes no sense at all.

The diff should be squashed into the previous commit and the change described,
if needed, should be implemented.

>   test: Refer to output filename by variable, not a hardcoded value

This could get "test, svg:" instead of just "test". Besides that, dunno. Never
looked closely at how SVG does things.

[...]

Sorry for complaining again (and sorry for being awfully slow for looking at
things! If you want to, feel free to just do something about patch 6/7 and
ignore the rest of my complaints, those are not that important).

Cheers,
Uli
-- 
Sent from my Game Boy.