Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for server error Mantle objects #31

Merged
merged 6 commits into from Feb 14, 2014
Merged

Conversation

alleus
Copy link
Contributor

@alleus alleus commented Feb 11, 2014

When errorResultClass is supplied, Overcoat attempts to instantiate an error object from the response if errors are returned from AFNetworking.

As discussed in #21

When errorResultClass is supplied, Overcoat attempts to instantiate an error object from the response if errors are returned from AFNetworking.
@gonzalezreal
Copy link
Contributor

Thanks for submitting the PR! I am not sure about the API changes though. I think it should be enough to have a single method to set the error class for all requests instead of adding the extra errorResultClass argument.

What do you think?

@alleus
Copy link
Contributor Author

alleus commented Feb 11, 2014

Ah, you're thinking like a property on the OVCClient? That's a good idea :) That way no APIs get changed for the requests.

@gonzalezreal
Copy link
Contributor

Yeah, I meant a property.

@gonzalezreal
Copy link
Contributor

Or perhaps an init method plus a read-only property.

@alleus
Copy link
Contributor Author

alleus commented Feb 11, 2014

Hum, that's true. I'm thinking this isn't something you'd want to change once the client is created (just like baseURL), that would be a big confusing.

@alleus
Copy link
Contributor Author

alleus commented Feb 11, 2014

There, made some changes. Take a look at it and see if it feels like something you want in this nice little module of yours. Also, I'm not too sure about the description in the method/property docs, feel free to let me know of any other changes you see fit! I'll get back tomorrow 😃

@gonzalezreal
Copy link
Contributor

Thank you! I'll have a look at the changes during the weekend.

return [self clientWithBaseURL:url account:account errorResultClass:nil];
}

+ (instancetype)clientWithBaseURL:(NSURL *)url account:(ACAccount *)account errorResultClass:(Class)errorResultClass {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not add + clientWithBaseURL:(NSURL *)url errorResultClass:(Class)errorResultClass?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed :)

@gonzalezreal
Copy link
Contributor

LGTM. Thanks Martin!

gonzalezreal added a commit that referenced this pull request Feb 14, 2014
Support for server error Mantle objects
@gonzalezreal gonzalezreal merged commit c3992d9 into Overcoat:master Feb 14, 2014
@alleus
Copy link
Contributor Author

alleus commented Feb 14, 2014

Yay! :bowtie:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants