Conversation
There was a problem hiding this comment.
LGTM — straightforward version bump with only a nit-level stale comment flagged inline.
Extended reasoning...
Overview
This is a mechanical release PR bumping the Flutter SDK from 5.5.0 to 5.5.1. Changes span five files: pubspec.yaml, android/build.gradle, ios/onesignal_flutter.podspec, and the SDK version strings in both the Android Java plugin and iOS Objective-C plugin. The Android native dependency is also updated from OneSignal 5.7.6 to 5.7.7.
Security Risks
None. There are no auth, crypto, permissions, or sensitive data handling changes. This is purely a version string and dependency version update.
Level of Scrutiny
Low. The change is entirely mechanical — every modification follows a consistent pattern of replacing '5.5.0' with '5.5.1' (or '050500' with '050501' for the encoded version strings). The Android SDK dependency bump (5.7.6 → 5.7.7) is a patch-level update documented as a Kotlin downgrade fix, which is low risk.
Other Factors
The only issue identified was a nit: the comment on Android line 26 still reads 'For 5.0.0' while the code now reflects 5.5.1. This is cosmetic and does not affect correctness or behavior. No prior reviews exist on this PR.
| // For 5.0.0, hard code to reflect SDK version | ||
| OneSignalWrapper.setSdkVersion("050500"); | ||
| OneSignalWrapper.setSdkVersion("050501"); |
There was a problem hiding this comment.
🟡 The comment on line 26 says '// For 5.0.0, hard code to reflect SDK version' but the code below it sets version '050501' (5.5.1), creating a mismatch that will persist with every future version bump. Consider updating the comment to reflect that this is an ongoing hard-coded pattern for the current Flutter wrapper version (e.g., '// Hard code to reflect the current Flutter wrapper SDK version').
Extended reasoning...
What the bug is: Line 26 of OneSignalPlugin.java has the comment // For 5.0.0, hard code to reflect SDK version immediately above OneSignalWrapper.setSdkVersion("050501"). The version string "050501" decodes to 5.5.1, not 5.0.0, creating an immediate contradiction between the comment and the code.
How it manifests: A developer reading this code sees "For 5.0.0" and looks at the value being set — "050501" — and must mentally reconcile why a comment references 5.0.0 while the value represents 5.5.1. The most natural reading is that the comment is wrong. It could mean: (a) this is a workaround that only applies to version 5.0.0 and should be removed now, (b) the intended value was "050000" and there is a regression, or (c) the comment just has not been updated.
The specific code path: In init(), the SDK type and version are set via OneSignalWrapper. This is the one place where the Flutter wrapper identifies itself to the native SDK. The version must stay in sync with the published Flutter package version (pubspec.yaml), but the comment does not reinforce that expectation — it mentions a historical launch version instead.
Why existing code does not prevent it: There is no enforcement mechanism tying the comment to the version string. When releasing a new version, the developer updates "050500" → "050501" (as done in this PR) but the comment referencing "5.0.0" is left untouched. The same pattern exists in the iOS counterpart (OneSignalPlugin.m) but without any such comment, so only the Android side carries this confusion.
Impact: The code is functionally correct — the value "050501" correctly represents 5.5.1 and is what gets transmitted to OneSignal's backend. However, the stale comment may cause a future maintainer to question whether the hard-coding is still intentional or whether a cleanup was forgotten, adding unnecessary cognitive overhead during releases or code reviews.
How to fix: Replace the comment with something that conveys the permanent intent rather than a historical origin. For example: // Hard-code to reflect the current Flutter wrapper SDK version. Alternatively, referencing where the source of truth lives (pubspec.yaml) would help: // Keep in sync with pubspec.yaml version.
Proof by example: This PR bumps the version from 5.5.0 to 5.5.1. The diff shows line 27 changing from "050500" to "050501" — but line 26's comment // For 5.0.0, hard code to reflect SDK version is untouched. A developer reviewing this diff would immediately notice the 5.0.0 ↔ 5.5.1 discrepancy and wonder if the comment should have been updated too, or if there is deeper significance to the 5.0.0 reference.
Channels: Current
🛠️ Native Dependency Updates