mapbox / mapbox-gl-native-ios

Interactive, thoroughly customizable maps for iOS powered by vector tiles and OpenGL
https://www.mapbox.com/mobile/
Other
210 stars 121 forks source link

Generate API diffs to better surface changes #180

Open friedbunny opened 5 years ago

friedbunny commented 5 years ago

Tools like apidiff exist to analyze frameworks and produce diffs of their APIs. We should apply this sort of tool in (at least) two ways:

1. Per pull request

Pull requests that change or add public API could have this information surfaced as a bot comment or GitHub check. This would help us identify subtle changes to public API that could be breaking.

2. Per release

We should generate and publish API diffs with our release documentation. It’s currently non-obvious in jazzy (and probably javadoc, too?) what code has changed in any given release, so a list of new/changed API would improve end-developers’ ability to upgrade confidently and quickly.

/cc @mapbox/maps-ios @mapbox/maps-android @1ec5

friedbunny commented 5 years ago
For instance, here’s the output for ios-v4.10.0 vs 9500430eeae5f07231872fc91f5b674defc66cbd (current release-mojito).
Auto-generated by running: apidiff ios-v4.10.0 head objc platform/ios/src/Mapbox.h #### MGLStringFromMetricType *new* sourcekitten.source.lang.objc.decl.function: `MGLStringFromMetricType` #### MGLMetricsManager *new* class: `MGLMetricsManager` *new* method: `-pushMetric:withAttributes:` in `MGLMetricsManager` *new* property: `sharedManager` in `MGLMetricsManager` *new* property: `delegate` in `MGLMetricsManager` #### MGLMetricType *new* enum: `MGLMetricType` *new* typedef: `MGLMetricType` *new* enum value: `MGLMetricTypePerformance` in `MGLMetricType` #### MGLLocationManager *new* method: `-distanceFilter` in `MGLLocationManager` *new* method: `-setDistanceFilter:` in `MGLLocationManager` *new* method: `-setActivityType:` in `MGLLocationManager` *new* method: `-desiredAccuracy` in `MGLLocationManager` *new* method: `-activityType` in `MGLLocationManager` *new* method: `-setDesiredAccuracy:` in `MGLLocationManager` *removed* property: `distanceFilter` in `MGLLocationManager` *removed* property: `activityType` in `MGLLocationManager` *removed* property: `desiredAccuracy` in `MGLLocationManager` #### MGLMetricsManagerDelegate *new* method: `-metricsManager:shouldHandleMetric:` in `MGLMetricsManagerDelegate` *new* method: `-metricsManager:didCollectMetric:withAttributes:` in `MGLMetricsManagerDelegate` *new* protocol: `MGLMetricsManagerDelegate` #### MGLSymbolZOrder *new* enum value: `MGLSymbolZOrderAuto` in `MGLSymbolZOrder` *modified* enum value: `MGLSymbolZOrderSource` in `MGLSymbolZOrder` | Type of change: | Swift declaration | |---|---| | From: | `case source = 1` | | To: | `case source = 2` | *modified* enum value: `MGLSymbolZOrderViewportY` in `MGLSymbolZOrder` | Type of change: | Swift declaration | |---|---| | From: | `case viewportY = 0` | | To: | `case viewportY = 1` | #### MGLSymbolStyleLayer *new* property: `symbolSortKey` in `MGLSymbolStyleLayer` #### MGLLoggingLevel *new* enum value: `MGLLoggingLevelDebug` in `MGLLoggingLevel` *modified* enum value: `MGLLoggingLevelError` in `MGLLoggingLevel` | Type of change: | Swift declaration | |---|---| | From: | `case error = 2` | | To: | `case error = 3` | *modified* enum value: `MGLLoggingLevelFault` in `MGLLoggingLevel` | Type of change: | Swift declaration | |---|---| | From: | `case fault = 3` | | To: | `case fault = 4` |

This required a small change to apidiff's sourcekitten invocation (-I platform/darwin/src/), but otherwise it worked pretty well out of the box. If this output were integrated into our jazzy docs and the methods/properties/etc. linked-up, this could be quite nice.

zugaldia commented 5 years ago

Thanks @friedbunny - I love this idea. I believe we have some prior art coming from Nav SDK.

/cc: @Guardiola31337

julianrex commented 5 years ago

Would this catch the nullable problems we had with the recent static analysis fix (and reported in https://github.com/mapbox/mapbox-gl-native/issues/14595)? i.e. is the full signature captured?

friedbunny commented 5 years ago

Would this catch the nullable problems we had with the recent static analysis fix (and reported in https://github.com/mapbox/mapbox-gl-native/issues/14595)? i.e. is the full signature captured?

For Apple platforms, if we use a tool based on sourcekitten/SourceKit, yes, this is something we can choose to monitor.

Update:

Here’s what a nullability change could look like — switching the view parameter’s nullable to nonnull here generates notices for both ObjC and Swift declarations.
*modified* method: `-[MGLMapView convertPoint:toCoordinateFromView:]` | Type of change: | Declaration | |---|---| | From: | `- (CLLocationCoordinate2D)convertPoint:(CGPoint)point toCoordinateFromView:(nullable UIView *)view;` | | To: | `- (CLLocationCoordinate2D)convertPoint:(CGPoint)point toCoordinateFromView:(nonnull UIView *)view;` | *modified* method: `-[MGLMapView convertPoint:toCoordinateFromView:]` | Type of change: | Swift declaration | |---|---| | From: | `func convert(_ point: CGPoint, toCoordinateFrom view: UIView?) -> CLLocationCoordinate2D` | | To: | `func convert(_ point: CGPoint, toCoordinateFrom view: UIView) -> CLLocationCoordinate2D` |
julianrex commented 4 years ago

cc @knov