Code review comment for lp:~malept/pyexiv2/0.2-ReadMetadataTestCase

Revision history for this message
Olivier Tilloy (osomon) wrote :

Thanks for the patch Mark! Overall it looks good to me.

I have one remark on the consistency of the checks: the original case tests the python type of the values of EXIF and IPTC tags. In the case you added, the check is on the tag's type, not the type of the corresponding python object.

Also, how about merging some selected XMP metadata from exiv2-bug540.jpg into smiley1.jpg using exiv2 itself, rather than doing the testing on two different files, and what's more on a very large number of XMP tags?

In fact I can see many ways to improve the original test case, but that's definitely out of the scope of your patch. If you can fix the check consistency issue, then I'll merge your branch, and take some time to further enhance the tests.

review: Needs Fixing (code)

« Back to merge proposal