Skip to content
This repository has been archived by the owner on Aug 30, 2023. It is now read-only.

Increase Swift support #68

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

fizker
Copy link

@fizker fizker commented Nov 29, 2015

I added nullability annotations and generic types for the dictionaries. This makes the compiler help you regardless of language, and makes the project much more usable from Swift.

I made some guesses on null and the <NSString *, NSString *> dictionaries based on the code and the tests, so there is a chance that the annotations are wrong ;).

The one downside is that it requires Xcode 7 to understand the annotations.

@dcramer
Copy link
Member

dcramer commented Nov 30, 2015

@fizker I think this is sane

For context on the dictionaries, and please confirm that this is correct in the code, both tags and extra can be null. tags should be a list of map[string][string] and extra should effectively be map[string][any].

@fizker
Copy link
Author

fizker commented Dec 1, 2015

Are you sure about extra and tags being nullable?

  1. The convenience constructors at https://github.com/getsentry/raven-objc/blob/master/Raven/RavenClient.m#L115-L121 pass in @{} instead of nil

  2. At https://github.com/getsentry/raven-objc/blob/master/Raven/RavenClient.m#L365 and https://github.com/getsentry/raven-objc/blob/master/Raven/RavenClient.m#L370, they are used as parameters to [NSMutableDictionary dictionaryWithDictionary:], which is marked as being nonnull by Apple.

    It does not seem to hurt passing nil to [NSMutableDictionary dictionaryWithDictionary:], but the docs does say that it should not be nil.

I will change the extra type to be NSDictionary<NSString*,NSObject*>.

@dcramer
Copy link
Member

dcramer commented Dec 3, 2015

I'm at least certain that the server endpoint accepts this as nullable or empty.

@fizker
Copy link
Author

fizker commented Dec 8, 2015

I am fine with the server being more lax than the client. I would actually prefer the dictionaries to be non-nil, as it makes the client API easier to work with. I see the nil (or missing) property for the web to be a bandwidth issue more than anything else.

@fizker
Copy link
Author

fizker commented Feb 24, 2016

Is there anything I can do to help this along?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants