docs: Add KSCrash migration strategy document #8094
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| # KSCrash Migration Strategy | ||
|
|
||
| Date: June 16th 2026 | ||
| Author: @NinjaLikesCheez <thomas.hedderwick@sentry.io> | ||
|
|
||
| ## Background | ||
|
|
||
| We're migrating from `SentryCrash` (a KSCrash v1.x fork with renamed identifiers) to KSCrash 2.x. The new integration (`SentryKSCrashIntegration`) is being built alongside the existing `SentryCrashIntegration`. We need a strategy for how these two coexist during development and how the cutover happens. | ||
|
|
||
| --- | ||
|
|
||
| ## Option A: Long-lived feature branch | ||
|
|
||
| Keep all KSCrash work on a dedicated branch (`kscrash-*`) and merge back to `main` in one big-bang PR when complete. | ||
|
|
||
| **Pros** | ||
|
|
||
| - No impact on `main` until work is done | ||
| - Freedom to iterate without gating concerns | ||
|
|
||
| **Cons** | ||
|
|
||
| - Long-lived branches diverge; merge conflicts galore | ||
| - The final merge PR is large, hard to review, and risky to ship | ||
| - Other teams (React Native, Flutter, Unity) have to swap branches to test | ||
|
Collaborator
There was a problem hiding this comment. I would also add, as a "con", that e2e testing requires continuous bidirectional sync, since the invariant of those tests should be that most tests should be able to run on both sides. Which is a considerable churn.
Member
Author
There was a problem hiding this comment. Good point - will add that shortly |
||
| - E2E testing will require bi-directional syncing between the integration branch and main | ||
|
|
||
| --- | ||
|
|
||
| ## Option B: Dual integrations on `main` (proposed) | ||
|
Member
There was a problem hiding this comment. My main concern with this approach is that we have to add KSCrash to the dependencies in Package.swift, and every user needs to check it out, even if they do not use it. There is no conditional compilation flag for Package.swift
Member
Author
There was a problem hiding this comment. Yup, that is the main downside (one I will note in the doc as well). In testing it's a ~3.3MB checkout with SPM which seemed negligible - if you know of a better way to integrate this (until the cut over) I'm happy to not add it as a direct dependency. |
||
|
|
||
| Ship both `SentryCrashIntegration` and `SentryKSCrashIntegration` on `main`. They are mutually exclusive at runtime — only one installs its crash handlers. Which one runs is controlled by two guards: | ||
|
|
||
| 1. `**#if SDK_V10` compiler flag** — `SentryKSCrashIntegration` is compiled into the binary only when building for V10. | ||
| 2. `**options.experimental.enableKSCrashIntegration`** — an opt-in flag (also gated behind `#if SDK_V10`) that must be `true` for `SentryKSCrashIntegration` to install. `SentryCrashIntegration` checks this flag and skips installation when the KSCrash path is active. | ||
|
|
||
| **Pros** | ||
|
|
||
| - Work ships incrementally to `main`; no merge cliff & conflict mess | ||
|
Collaborator
There was a problem hiding this comment. I would say "no merge cliff" or "'flip the switch' change" are only true if the KSCrash-related CI is fully integrated on main and blocks PRs, if something broke. Otherwise, this still relieves us from continuous sync efforts, but we defer the pain.
Member
Author
There was a problem hiding this comment. That is true however, v10 code is already expected to be buildable by the CI and would block PRs merging. The testing side currently isn't integrated, but by the time the work is set to be delivered this should be integrated and blocking. Do you mean something else I've not considered?
Collaborator
There was a problem hiding this comment.
Sure, buildable is great, but far from "no merge cliff". For "no merge cliff", I would expect not only builds but all tests to run for v10 and block PRs. |
||
| - Hybrid SDK consumers can test against the KSCrash path and swap between the two more easily | ||
| - The cutover becomes a 'flip the switch' change | ||
| - Easier code review — changes land in small, reviewable chunks | ||
|
|
||
| **Cons** | ||
|
|
||
| - Both integrations live in the codebase simultaneously for a period | ||
|
Collaborator
There was a problem hiding this comment. I think this is not a "con" in its own right. That is just a fact of this option. However, regarding "merge cliff" etc., an essential con is that if this should really be integrated early and provide feedback, PRs for v9 might be blocked by tests that cover a rather global invariant for v10. We could express this as a separate option "C" or an option to the option, but the crux with option B is: do we want to integrate early so we have "no merge cliff" and "no conflict mess", or are we happy with solely "no conflict mess". The former means PR blocking CI for V10, the latter means V10 being colocated on the same branch.
Member
Author
There was a problem hiding this comment. Understood - I'll schedule some time this week to think about this aspect. Maybe it would be acceptable to the team to skip SentryCrash tests in v10 mode. Otherwise I'll incorporate this feedback into the document - thanks!
Collaborator
There was a problem hiding this comment. Just to be clear: this doesn't have to be a biggie or even an up-front decision; it should just calibrate expectations, and our written assumptions should reflect that. If we are interested in flipping the switch, then the test battery must run continuously. Even if v9 PRs aren't outright blocked, there should be something obviously red enough for us to act on (that is what I meant yesterday in our sync). If we only colocate and build together to primarily reduce the churn from continuously syncing two branches, then we should be aware that we do not have feedback on whether main v9 and v10 drift on global invariants (some of which are not expressible via interfaces or types in general). And that means we must allocate some time after flipping the switch. |
||
| - `#if SDK_V10` guards add a small amount of conditional-compilation mental noise for developers | ||
| - SPM users will checkout KSCrash as a dependency, whether it's used or not | ||
|
|
||
| --- | ||
|
|
||
| ## Proposal: Option B | ||
|
|
||
| The dual-integration approach is lower risk and produces a better end result. The `SDK_V10` flag already exists in the build system. | ||
|
Member
There was a problem hiding this comment. If we accept the downside of having KSCrash as a dependency even if we don't use it yet (gated by feature flag), I would also go with this option so we can do releases with partial functionality (if applicable) and have a full V10 testing setup on |
||
|
|
||
| **Cutover plan (when KSCrash integration is feature-complete):** | ||
|
|
||
| 1. Remove `enableKSCrashIntegration` | ||
| 2. Remove the `#if SDK_V10 guards` | ||
| 3. Update SentryKSCrashIntegration to use the traditional `enableCrashHandler` option as it's enable guard | ||
| 4. Remove SentryCrash altogether | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
m: I believe this might be better suited to live inDECISIONS.mdThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, once a decision is made - would it be better to open this for discussion as an issue and close this out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can keep this doc open and collapse it into a decision at the end.