-
-
Notifications
You must be signed in to change notification settings - Fork 410
docs: Add KSCrash migration strategy document #8094
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
087abbe
docs(KSCrash): add migration strategy document
NinjaLikesCheez b8f44c2
doc: add downside of early SPM approach
NinjaLikesCheez eec0e31
docs: add e2e testing pains to the cons list
NinjaLikesCheez 0f0e241
docs: remove SDK_V10 flags in favour of ENABLE_KSCRASH
NinjaLikesCheez 70f48e7
docs: add env var guard, remove related con
NinjaLikesCheez 95d5afa
docs: add some clarity on when this is intended to ship, and that Sen…
NinjaLikesCheez 6641b34
docs: add scoping section
NinjaLikesCheez 88362f2
docs: convert migration doc to decision
NinjaLikesCheez File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| # 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 | ||
|
NinjaLikesCheez marked this conversation as resolved.
Outdated
|
||
|
|
||
| --- | ||
|
|
||
| ## Option B: Dual integrations on `main` (proposed) | ||
|
NinjaLikesCheez marked this conversation as resolved.
Outdated
|
||
|
|
||
| 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 | ||
|
NinjaLikesCheez marked this conversation as resolved.
Outdated
|
||
| - 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 | ||
|
NinjaLikesCheez marked this conversation as resolved.
Outdated
|
||
| - `#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 | ||
|
NinjaLikesCheez marked this conversation as resolved.
Outdated
|
||
|
|
||
| --- | ||
|
NinjaLikesCheez marked this conversation as resolved.
Outdated
|
||
|
|
||
| ## 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. | ||
|
NinjaLikesCheez marked this conversation as resolved.
Outdated
|
||
|
|
||
| **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 | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.