Skip to content

Commit

Permalink
[IOSP-647] Add documentation for Danger HealthKit rule (#378)
Browse files Browse the repository at this point in the history
  • Loading branch information
Olivier Halligon authored Apr 28, 2020
1 parent d8eb7c3 commit 020439d
Showing 1 changed file with 31 additions and 0 deletions.
31 changes: 31 additions & 0 deletions Cookbook/Technical-Documents/DangerRules.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ For a more in-depth presentation about Danger and how it works, see [these slide

The [Danger website](https://danger.systems/ruby/) also has a pretty detailed documentation (especially look at the links to list of Guides and the DSL reference at the very bottom of the page).


## Danger documentation updates

> * Declared in: `danger-ci/danger.rb`
Expand All @@ -22,6 +23,7 @@ The [Danger website](https://danger.systems/ruby/) also has a pretty detailed do
This rule simply reminds us to update this very documentation when the Danger configuration is changed (when any file in `danger-ci/*` is modified), so that we keep the documentation in sync with the actual rules.


## PR Size

> * Declared in: `danger-ci/pr_size.rb`
Expand All @@ -39,6 +41,7 @@ If this rule is triggered, an table is also added showing some stats (the table

This rule is only a warning because we don't want it to block PRs, as there are some rare exceptions where we allow the limit to be exceeded, as long as it's justified by the author (e.g. a refactoring PR)


## Podfile Checks

> * Declared in: `danger-ci/pods.rb`
Expand All @@ -63,6 +66,7 @@ This rule is just to inform (informative message, no error nor warning) when the

The goal is to suggest them to take extra care in reviewing why we needed those changes in our dependencies and if they were properly justified.


## SDK: Mark changes to any of our SDKs with `#SDK` hashtag

> * Declared in: `danger-ci/sdk.rb`
Expand All @@ -78,6 +82,7 @@ The hashtag is only needed for Pull Requests addressing tickets from non-SDK boa

_This is needed because of the Change Request Process (CRP) from our sSDLC requirement: any ticket touching code in the SDK needs to be included in the CRP ticket for next SDK release, and that hashtag will allow us to include those tickets automatically._


## pbxproj checks

> * Declared in: `danger-ci/pbxproj.rb`
Expand Down Expand Up @@ -124,6 +129,7 @@ This rule will warn you if any part of the pbxproj references an UUID that got a

When that happens, you need to figure out if the removal of the file was intended as part of your PR (and if so, fix the merge by also removing the lines pointing to that now-deleted file reference), or if the removal of the file from the pbxproj was unintented (and if so, restore it). You could do that by editing the `pbxproj` manually, but if possible it's even better if you can fix it in Xcode (e.g. by removing the offending file from the project and add it again)


## Detect Missing Feedbacks

> * Declared in: `danger-ci/feedbacks.rb`
Expand All @@ -139,6 +145,31 @@ Note that this rule can have false positives in rare cases. Especially if you us

(It still detects `Feedbacks` called conditionally though – like `if (cond) { feedbacks += Self.whenBlah() }` – and should still catch the most common cases of forgetting to add a newly-created `Feedback` to the state machine)


## Check HealthKit configuration consistency

> * Declared in: `danger-ci/healthkit.rb`
> * Function: `check_healthkit`
> * Type: 🚫 failure
When configuring HealthKit on a target, either to enable it on a given app flavor or to disable it if we decide to remove the feature for a flavor or for a new target we cloned from an existing one, we need to configure it in multiple places in the project and target, and ensure those configurations are consistent.

If there are some inconsistencies in the way HealthKit is configured in the different places in a target, we will likely get rejected by Apple when submitting on the AppStore, especially because we could think that we disabled it but Apple will still find traces of it, or we enabled it but forgot to turn some flags on affecting some `#if` in our Swift code depending on it. This is why this rule exists, to ensure everything is consistent and we don't get rejected last minute for HealthKit configuration issues.

Every time the `Babylon.xcodeproj` file is modified, this rule will iterate on each target and each build config (Debug/Release) and for each will check the following places for the HealthKit setup:

* Is HealthKit enabled in the Entitlements files?
* Are the 2 keys `NSHealthShareUsageDescription` and `NSHealthUpdateUsageDescription` both set in Info.plist?
* Is `HEALTHKIT_INTEGRATION_AVAILABLE` flag turned on in Build Settings under `SWIFT_ACTIVE_COMPILATION_CONDITIONS`?
* Is `HealthKit.framework` linked with the target?
* Is `BabylonHealthKitIntegrationSDK.framework` linked with target?

If the answer to these questions for a given target and build configuration are not all the same (not all `true` nor all `false`), this means that there is an inconsistency in the HealthKit setup for this target and build config.
In that case, for each target+config, the rule will report a failure, and also add a markdown table listing the state of the HealthKit setup (✅/❌) for each of those 5 setup places for that target+config.

> See also [this documentation in our private repo](https://github.com/babylonhealth/babylon-ios/blob/develop/Documentation/importing-healthkit/HowToCorrectlySetHealthKitPermissionsInAnAppWhenEnablingHKFunctionality.md) to learn how to configure HealthKit in our projects.

## swiftlint

> * Declared in: `danger-ci/swiftlint.rb`
Expand Down

0 comments on commit 020439d

Please sign in to comment.