Skip to content
This repository was archived by the owner on Sep 16, 2022. It is now read-only.

Conversation

@aneshas
Copy link

@aneshas aneshas commented Dec 8, 2015

Added nested struct validation and more descriptive req error message.
Closes #107

Signed-off-by: Anes Hasicic [email protected]

Added more descriptive req error message
Reordered req check in order to fix, failing tests

Signed-off-by: Anes Hasicic <[email protected]>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure passing the parent field is really worth it

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I passed in the parent because I added return fmt.Errorf("missing field %v in %v", t.Name, parent.Name()) a parent to a missing field error.
I did it because if you have multiple fields with the same name in a struct and nested structs, the default missing field %v felt non descriptive, you wouldn't know which struct it belongs to. But if you feel like it's too much, I will remove it :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the comment on line 760

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, but I don't understand what that comment should point out me?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That comment is about adding the info about the parent type on the

if err != nil { return ... } 

@campoy
Copy link
Contributor

campoy commented Dec 8, 2015

This is pretty good, but please add tests and address my comments.

Thanks!

@aneshas
Copy link
Author

aneshas commented Dec 8, 2015

@campoy Thanks, yes I will add tests

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't pass the type and instead just add the info to the error message.

return fmt.Errorf("%v in %s", err, t)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants