sean7512 / RestEssentials

Lightweight REST and JSON library for Swift.
MIT License
37 stars 12 forks source link

Added PATCH call to perform partial changes to a resource #3

Closed ThomsonIT closed 8 years ago

ThomsonIT commented 8 years ago

I added methods to PATCH a resource. Maybe you want to merge this?

sean7512 commented 8 years ago

@ThomsonIT I see no reason not to add this..do you need a new release with it in cocoapods as well? Otherwise, I'll probably merge it and wait until I get around to supporting the new Swift Package Manager.

ThomsonIT commented 8 years ago

I don't need a new release. Sounds good to support the Swift Package Manager. I'm planning to add more functions as well. I want to keep it clean and simple as you did but would like to add the ability to react to different http status codes. The Rest API i am using is responding with the corresponding codes to make sure if the triggered action worked or mark the reason why it didn't.

sean7512 commented 8 years ago

Yes, I want to ensure this stays as drop dead simple as possible. I made it because AlamoFire was too large/did too much. I only want this to focus on the standard HTTP methods and JSON.

May I suggest we make expectedStatus code an Optional (Default to 200 as it is now). If you override it with nil, then any status code returned will allow the call to pass. Then we can pass the status code back through the callback. Then each implementation can deal with the status code as they wish.

That may be a simpler/cleaner implementation than adding the ability to react to status codes within the framework.

Thoughts?

ThomsonIT commented 8 years ago

Sounds good that way. Are you going to implement this?

sean7512 commented 8 years ago

See branch: feature_request/allow_any_status_code_let_callback_handle

The only thing I do not like is that now to get the JSON returned it is:

let json = try result.value().json

I don't like the added .json, as it isn't as elegant as before. It could also be passed back as:

(Result<JSON>, NSHTTPURLResponse?)

Thoughts?

ThomsonIT commented 8 years ago

I dont mind using it either way. Presumably i would use the second solution.

sean7512 commented 8 years ago

Sorry, been really busy...I will implement the 2nd solution soon and then we can merge this