Skip to content

Conversation

@0bmxa
Copy link

@0bmxa 0bmxa commented Jun 8, 2020

Updated with changes for v5.44.0.

I'm not very experienced with typescript and typings, so I essentially copied how the other typings looked. npm run test and npm run lint run without errors.

I had a few issues, where I'm curios how you'd have done that:

  • many methods/properties had no description available in Apple's docs, so I didn't know what to add here. Would you prefer to make them up or just leave them empty?
  • setCameraBoundaryAnimated accepts two different types as its first parameter, but according to Apple's docs they use a different parameter name depending on the type (coordinateRegion: mapkit.CoordinateRegion | mapRect: mapkit.MapRect), which (as far as I understand – please correct me if I'm wrong) is not supported in typescript. I tried overloading the method, i.e. adding it twice (one for each parameter name), but the linter insisted on combining them, and I wasn't sure if silencing the linter is more desirable, so I changed it.
    See the docs here and the line in my code here.
  • I also noticed, several properties being marked as optional. I also did that where test would fail because of them not being optional, but I didn't really understand where that is appropriate. Should that be done everywhere?

I'm happy about feedback, or also happy to change anything, just let me know!

@wsmd
Copy link
Owner

wsmd commented Jun 8, 2020

Thanks for the PR @0bmxa! I'll take a look and get back to you very soon!

@treemmett
Copy link

@wsmd Any chance this could get a review?

@andreicocari
Copy link

Bump?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants