remove panics in favor of errors

[rwcarlsen]

API changes include:

• remove all explicit panics in favor of error returns.
• rename TypeCategory -> Format
• change String() method to only return tag value and without brackets if Count == 1

[fawick]

Looks good to me. Thanks for making this effort!

What's the history behind the type Format (ex-TypeCategory)? I saw that the goexif-version over cat camlistore calls it FormatType but I have a hard time identifying the commit in which they started using that name. Do you know anything about that?

[fawick]

Nevermind, in the end I could find it myself. https://code.google.com/p/camlistore/issues/detail?id=498#c6

[fawick]

How do you want to proceed with this? Are there any particular reasons for why you are hesitating to merge it to upstream?

[rwcarlsen]

I'm just sitting on it for a few days to make sure there aren't any other (api) changes I or others might want.

On Tue, Sep 2, 2014 at 1:16 PM, Fabian Wickborn notifications@github.com wrote:

How do you want to proceed with this? Are there any particular reasons for why you are hesitating to merge it to upstream?

— Reply to this email directly or view it on GitHub https://github.com/rwcarlsen/goexif/pull/24#issuecomment-54194258.

[fawick]

There are some contributions to goexif that happened over at Camlistore which I am planning to port to this repo in form of a Pull Request. As the nopanic branch is still pending I wonder whether you would prefer me to provide the PR against the current HEAD go1 or, alternatively, against nopanic directly? I'm fine with any preference you might have, but mind you, there is going to be a merge conflict between nopanic and what I'm going to propose which needs to be resolved manually. I'd be happy to do that work on my end.

[rwcarlsen]

Why don't you PR into the nopanic branch.

On Mon, Sep 15, 2014 at 11:21 AM, Fabian Wickborn notifications@github.com wrote:

There are some contributions to goexif that happened over at Camlistore which I am planning to port to this repo in form of a Pull Request. As the nopanic branch is still pending I wonder whether you would prefer me to provide the PR against the current HEAD go1 or, alternatively, against nopanic directly? I'm fine with any preference you might have, but mind you, there is going to be a merge conflict between nopanic and what I'm going to propose which needs to be resolved manually. I'd be happy to do that work on my end.

— Reply to this email directly or view it on GitHub https://github.com/rwcarlsen/goexif/pull/24#issuecomment-55615611.

[fawick]

Okay, understood, against nopanic indeed it is.

[fawick]

Is regress_expected_test.go up-to-date with respect to 1335a39?

[rwcarlsen]

Yes - at least I think I got the tests adapted correctly.

On Mon, Sep 15, 2014 at 3:55 PM, Fabian Wickborn notifications@github.com wrote:

Is regress_expected_test.go up-to-date with respect to 1335a39 https://github.com/rwcarlsen/goexif/commit/1335a39029ae69ebe2c8039af4b8895966a5b521?

— Reply to this email directly or view it on GitHub https://github.com/rwcarlsen/goexif/pull/24#issuecomment-55657237.

[fawick]

Strange. I needed to regen the file as I added a test image. Now I get a lot of failures like:

exif_test.go:154:    field XResolution bad tag: expected '"72/1"', got '{Id: 11A, Val: ["72/1"]}'

I'm on nopanic tip:

$ git show
commit 362b247035474c8165593e0378cc38b9364f6761

Steps to reproduce: a) Change regenRegress to true b) $ go test c) Change regenRegress back to false d) $ go test

EDIT: Following the hint to comment func (w *walker) Walk(field FieldName, tag *tiff.Tag) error out during regeneration has no effect on the outcome.

[rwcarlsen]

I think I know what's happening, I can get a fix in within the next day or so.

[fawick]

Yep, I found it too. The string that tiff.Tag.String() is producing does no longer match to the one that is build as got in func (w *walker) Walk(field FieldName, tag *tiff.Tag)

I can come up with a PR for this quickly, I've got my hands deep in the code anyway.

[fawick]

Ok, a fix for the regressions tests is in #26. The contributions coming from camlistore are in #27 (also containing the changes of #26).