[PROD-835] Push notifications

[jameshfisher]

No description provided.

[hamchapman]

I'm not familiar with the self-> syntax and I'm not really sure what it's doing.

Typically I've seen init methods written more like this: https://github.com/pusher/libPusher/blob/master/Library/PTPusherAPI.m#L23-L32

where you just do something like urlSession = [NSURLSession sharedSession]

[hamchapman]

Looks good in general.

Mainly just questions about the self->... syntax.

Also as a stylistic comment, I'd always leave an asterisk attached to a variable name when it's a pointer, i.e. NSString *myString... instead of NSString * myString...

[hamchapman]

Also looks like the PTNativePusher file might not be being added to the test targets (or something like that) and so that's why the tests are failing. I can look at that if you're unsure - just let me know 👍

[jameshfisher]

@hamchapman the self->foo syntax is ordinary C dereference, it's the same as (*self).foo - it's one of the few things I understood so I used it liberally :D

AFAIK you can just leave out self-> and it's semantically the same; it's like this. in Java where you can leave it out. I can just drop it though IMO it makes it clearer whether something is an instance var and when it's a local var

[jameshfisher]

@hamchapman * next to the variable?! You're a madman, it's part of the type! But I shall follow the house rules

[jameshfisher]

@hamchapman looks like you're right about the test failure but I'm not sure how to fix it https://travis-ci.org/pusher/libPusher/jobs/147807341

[hamchapman]

I made the tests pass by just restarting them a few times. Travis is a bit flaky with making the connection to the simulator device that's needed to run the tests so they fail occasionally.

I'll see if there's a fix for that today, but essentially all the tests are passing

[jameshfisher]

@hamchapman ha, great :)

Is there anything else I should do? I'm aware I don't have any testing here beyond manually verifying that the Sample App works. If we want to unit test, I don't know what the Obj-C way is - we'll want some HTTP client mocking of some sort.

[hamchapman]

Yeah there are some examples already in the library that you can follow. High level overview is that you'll want to use OHHTTPStubs for the HTTP mocking and then the testing framework being used is called Kiwi. Pretty standard stuff, so if you're up for it, go to town 👍

[jameshfisher]

alrighty, will do

[hamchapman]

Were these added by you manually or were they added by a cocoapods install / Xcode build settings update?

[jameshfisher]

ohhhhhhhheeeeeerrrrrrrrrrrrrrmmmmmmmmmmhmm I did add that second line when the internet told me to because something called XCTest.h was not found when building. But I later discovered that the problem was that my test file was somehow added to the library, and not just to the tests. So these lines probabably don't need to change here after all.

[hamchapman]

Yeah I'd be very hesitant to manually change any of the build settings of the Pods xcodeproj file 🎱

[jameshfisher]

Oh I only "manually" changed it via the magic UI which changed the file for me 🌟

[hamchapman]

All looking good to me 👍

[lukeredpath]

Sorry I'm a bit late on this. I need some time to look at this properly as I'm on holiday but my initial thoughts are that I don't want to introduce any major functionality until v2 and I'd also like to start moving away from Objective C and a good start would be to start writing new classes in Swift where possible.

[hamchapman]

We need to do some sort of a release (it doesn't need to be a 1.7, although that would be my preference) just to make sure that customers wanting to use the new native push feature that we've released (https://pusher.com/docs/push_notifications) can do so in their iOS (Objective-C) apps.

In terms of it being Swift vs Obj-C I'll admit that I'm not clued up enough to know about the implications of using both in a library. Would we need to swap to using dynamic frameworks (as mentioned in #219)?

[hamchapman]

Closing in favour of #222