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

TMTumblrSDK 2.0 #66

Open
irace opened this issue Aug 27, 2014 · 11 comments
Open

TMTumblrSDK 2.0 #66

irace opened this issue Aug 27, 2014 · 11 comments
Labels

Comments

@irace
Copy link
Contributor

irace commented Aug 27, 2014

I don't know when exactly I'll be getting around to it, but this SDK has gotten a little stale and in need of quite a bit of modernization. I'm hoping to have some time over the next few months to work on a rewrite, which I'll call TMTumblrSDK 2.0.

There are lots of goals, but some of the larger ones are:

  • Replace JXHTTP with AFNetworking: There are no plans for JXHTTP to be updated to take advantage of NSURLSession, and I'm going to choose AFNetworking over rolling my own in the interest of leveraging its multipart file upload functionality. AFNetworking will be an implementation detail and not exposed via the TMTumblrSDK API, which should result in fewer breaking API changes going forward.
  • Reduce API surface area: as documented here, there are multiple ways of making all Tumblr APIs currently. It's confusing and I want to get rid of it.
  • Better documentation for usage without CocoaPods: I think you should use CocoaPods for installing TMTumblrSDK but I still want to help you if you disagree.

Please feel free to add your thoughts here, or in a separate issue that you can link to in a comment here. I'm very receptive to feedback.

@irace irace added the question label Aug 27, 2014
@mbbischoff
Copy link
Contributor

I’d personally like to see simple value objects for requests and responses that’d we'd keep up to date with the API keys etc. Maybe protocol based so you could use them or not.

@irace
Copy link
Contributor Author

irace commented Aug 27, 2014

@mattbischoff Do you think it's important to provide a way for the client to get the raw URL dictionary as well? I suppose the model object could have a dictionaryRepresentation or rawValue method on it. Do you think it's overkill to have an API client have something like Mantle as a dependency? I wouldn't want Mantle objects to leak out into the public API, so maybe something like KZPropertyMapper would be a better fit.

@samdmarshall
Copy link

To put it out there, not everyone can use cocoapods on projects. I've come across specific client restrictions before that require our build system to be complete transparent to them for when the project gets transferred back to them after we complete it. The disagreement of cocoapods vs submodules aside, not providing the ability to use official SDKs is a bit of a disservice in non-proprietary ways can be challenging.

Also, I normally question the usage of adding networking library dependencies to an abstraction of a rest API. That raises a red flag for me in terms of what your API is actually doing for me that i couldn't do on my own to fulfill my own requirements. Before now i've never head of JXHTTP, but based on the current SDK code i'm not sure what AFNetworking would do over what you have already. I just looked at the SDK this morning when mentioned on twitter, so I do realize i don't have a full grasp of the extent and capabilities it has. That said, i'd be cautious to throw more code at the problem in the hopes it goes away. If you are using these libraries just for auth purposes and a clean set of network calls, maybe consider internalizing that rather than adding yet another heavy dependency?

@segiddins
Copy link
Contributor

Mantle would be overkill for a public library, imho. I can see both the benefits and drawbacks of vending model objects vs. dictionaries, namely stronger typing and more obvious usage (i.e. I wouldn't have to go to the Tumblr API docs to figure out how to get a blog's name). On the other hand, it would likely be more restrictive for those who would like to integrate Tumblr data into any system that isn't plain-old NSObjects.

@irace
Copy link
Contributor Author

irace commented Aug 27, 2014

not providing the ability to use official SDKs is a bit of a disservice in non-proprietary ways can be challenging

Totally fair. You can definitely use TMTumblrSDK without using CocoaPods, but you'd have to figure out how to do so yourself. We can do better.

Before now i've never head of JXHTTP, but based on the current SDK code i'm not sure what AFNetworking would do over what you have already.

I would like to use NSURLSession and expose NSURLSessionTask objects, which would allow TMTumblrSDK to more easily be used in say, an iOS 8 extension. JXHTTP was developed semi-internally (Justin Ouellette worked at Tumblr while he was writing it) so it was a good fit, but he's not updating it to add NSURLSession support, so we must move forward.

That said, i'd be cautious to throw more code at the problem in the hopes it goes away. If you are using these libraries just for auth purposes and a clean set of network calls, maybe consider internalizing that rather than adding yet another heavy dependency?

The main reason for using a existing networking library rather than rolling my own code is support for multipart file uploads. It may be overkill to include a whole library instead of just some code to handle this – I'll investigate the feasibility of breaking some out from either AFNetworking or JXHTTP, but to be honest, I kind of feel like AFNetworking is enough of a standard at this point that it's acceptable to include it as a dependency.

@endocrimes
Copy link

I agree with @segiddins, adding Mantle is a little bit of overkill for an SDK, although vending a model object of some sort has a multitude of advantages over an NSDictionary in terms of typing and ease of use. I would say that KZPropertyMapper could be a good fit, and is lightweight enough to drop in.

I can't say I'm a huge fan of AFNetworking and you can do multipart NSURLSessionUploadTasks support relatively easily AFAIK. (you could possibly borrow a few ideas from: https://github.com/mxcl/OMGHTTPURLRQ/blob/master/OMGHTTPURLRQ.m#L79)

@irace
Copy link
Contributor Author

irace commented Aug 27, 2014

you could possibly borrow a few ideas from: https://github.com/mxcl/OMGHTTPURLRQ/blob/master/OMGHTTPURLRQ.m#L79

Yeah, this reads it all into memory at once which I would avoid, but the point is valid. I could extract code from JXHTTP or AFNetworking to do this via streams.

The model object consideration is an interesting one. On one hand, we wouldn't even use them in the Tumblr app, the primary consumer of this library, since we use Core Data. We could obviously translate the vended objects into our Core Data models, the way we currently translate from NSDictionary, just feels weird to add the extra layer of abstraction. But I get the arguments for it too.

@brianmichel
Copy link
Contributor

Personally, I think an NSURLSession based approach might be really nice. Judging from Mattt's direction with AlamoFire there isn't a huge need for what AFNetworking offers anymore since the inclusion of the session based APIs.

@endocrimes
Copy link

@brianmichel That would be the preferred option from a potential users standpoint

@mrkev
Copy link

mrkev commented Jul 28, 2015

Any updates on this?

@irace
Copy link
Contributor Author

irace commented Jul 29, 2015

A ground-up rewrite on top of NSURLSession is still planned but we don’t have anything to share at the moment regarding when it might be ready.

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

No branches or pull requests

7 participants