search

kas-kad / AdvancedNSOperations

Objective-C implementation of concepts introduced at WWDC 2015 Session 226 Advanced NSOperations.
53 stars 13 forks source link

Your Code Implemented #4

Closed mazz closed 9 years ago

mazz commented 9 years ago

Hi again, I created the repo:

https://github.com/mazz/conditional-network-objc.git

Its intent is to implement the minimum set of classes to download the earthquakes JSON.

Also, it uses some of your code. I've added nullability qualifiers to the classes I used and I'll soon add those to your repo too.

But I have a request: would you mind helping me debug why it is not queuing the operations(it seems)?

kas-kad commented 9 years ago

At first sight, looks like you didn't implement a state observation like this https://github.com/purrrminator/AdvancedNSOperations/blob/master/Operations/Operations/KADOperation.m#L21

so your operations never get ready for executing

mazz commented 9 years ago

Good eye, thanks.

I actually added DownloadEarthquakesOperation to my fork of your code. Same issue. ReachabilityCondition is a no-op but I don't think that should matter.

I'm baffled.

kas-kad commented 9 years ago

Will check it. Might be some bugs in my group operation subclass...

kas-kad commented 9 years ago

I actually use KADOperations with custom conditions in my real project, work like a charm. Although didn't implement group operations yet.

mazz commented 9 years ago

OK, I will try my fork again, but attempt with a single KADOperation, not a KADGroupOperation and see if that works.

mazz commented 9 years ago

So I ran my fork with just a single KADOperation and I got it to 'work' by explicitly calling -resume on the NSURLSessionTask.

With the above tweak I tried the DownloadEarthquakesOperation KADGroupOperation. Still seems to not do anything though I didn't investigate the root cause(yet).

So it's possible there is something wrong with KADGroupOperation but I'm not sure.

Also cleaned-up KADOperation to use the more readable prefixed nullability qualifiers, and removed the small amount of generics code to allow for compilation on clang 6.x.

mazz commented 9 years ago

Hey there, did you get busy?

kas-kad commented 9 years ago

@mazzy yep, I hope I'll back in this weekend

kas-kad commented 9 years ago

Did you fixed that thing from my comment? https://github.com/purrrminator/AdvancedNSOperations/issues/4#issuecomment-121370902 I still can't see any keyPathsForValuesAffectingValueForKey in your Operation.m

kas-kad commented 9 years ago

Also not implemented Operation -()willEnqueue Why don't you use my base classes? Sorry but I have no time to review your code any further

mazz commented 9 years ago

Sure, will look into your comments.