Skip to content

Conversation

@maratal
Copy link
Collaborator

@maratal maratal commented Nov 10, 2025

Summary by CodeRabbit

  • Bug Fixes

    • Fixed crash when receiving HTTP 404 responses over HTTPS.
  • Chores

    • Package version bumped to 1.2.42.
    • Underlying native dependency versions updated.
    • iOS minimum deployment target raised to 12.0.
  • Documentation

    • Installation instructions and changelog entry updated for 1.2.42.

@maratal maratal requested a review from ttypic November 10, 2025 16:26
@coderabbitai
Copy link

coderabbitai bot commented Nov 10, 2025

Walkthrough

This PR updates the package to v1.2.42: bumps ably-cocoa to 1.2.53, raises example iOS deployment target to 12.0, adds several Firebase/Google frameworks to the example Xcode project embedding configuration, and documents a fix for a crash when HTTPS responses return 404.

Changes

Cohort / File(s) Summary
Release & Docs
CHANGELOG.md, README.md, pubspec.yaml
Bumped package version to 1.2.42; updated README installation example to ^1.2.42; added changelog entry listing ably-cocoa update and 404 HTTPS response crash fix.
iOS Podspec & Podfile
ios/ably_flutter.podspec, example/ios/Podfile
Updated CocoaPod dependency: Ably from 1.2.331.2.53; raised example iOS deployment target from 10.0 → 12.0.
Example Xcode project
example/ios/Runner.xcodeproj/project.pbxproj
Added embedded frameworks/input-output paths for Firebase and Google-related frameworks: FirebaseCore, FirebaseCoreInternal, FirebaseInstallations, FirebaseMessaging, GoogleDataTransport, GoogleUtilities, FBLPromises, Toast, nanopb.
iOS native code
ios/Classes/codec/AblyFlutterReader.m
Replaced [[ARTStringifiable alloc] initWithString:value] with class factory [ARTStringifiable withString:value] in tokenParamsFromDictionary; no other logic changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Check version consistency across pubspec.yaml, CHANGELOG.md, README.md, and ios/ably_flutter.podspec.
  • Verify CocoaPods dependency change (Ably1.2.53) builds cleanly and addresses the documented 404 crash.
  • Confirm raising deployment target to iOS 12.0 doesn't break example project targets or CI.
  • Review added frameworks in project.pbxproj for correctness (input/output paths, duplication, embedding settings).
  • Inspect the small Objective‑C factory replacement for correctness and memory/behavior parity.

Poem

🐇 A tiny hop to 1.2.42, so spry,
CocoaPod aligned, now reaching sky-high,
iOS climbs to twelve, frameworks join the run,
A 404 crash quelled — the fix is done. ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'Release/1.2.42' directly corresponds to the changeset, which bumps the package version from 1.2.41 to 1.2.42 and includes all associated updates (changelog, dependencies, iOS deployment target, and Firebase framework integrations).
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch release/1.2.42

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between d5195f6 and 51cbb34.

📒 Files selected for processing (1)
  • ios/Classes/codec/AblyFlutterReader.m (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: android (24)
  • GitHub Check: android (29)
  • GitHub Check: ios
  • GitHub Check: android
  • GitHub Check: ios
🔇 Additional comments (1)
ios/Classes/codec/AblyFlutterReader.m (1)

256-256: Confirm factory method withString: exists in ably-cocoa 1.2.53.

The change at line 256 replaces the direct initializer with a factory method. Web searches don't explicitly confirm this API exists in 1.2.53. Ensure the code compiles successfully with the updated SDK version and that this factory method is the correct API for this version.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot temporarily deployed to staging/pull/586/features November 10, 2025 16:27 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/586/dartdoc November 10, 2025 16:28 Inactive
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
example/ios/Podfile (1)

37-47: Critical: Inconsistent iOS deployment target in post_install hook.

Line 44 sets IPHONEOS_DEPLOYMENT_TARGET = '10.0', which conflicts with the platform declaration at line 2 (platform :ios, '12.0'). This override will cause generated projects to target iOS 10.0, negating the platform update.

Apply this diff to align the deployment target:

       project.targets.each do |target|
         target.build_configurations.each do |config|
-          config.build_settings['IPHONEOS_DEPLOYMENT_TARGET'] = '10.0'
+          config.build_settings['IPHONEOS_DEPLOYMENT_TARGET'] = '12.0'
         end
       end
🧹 Nitpick comments (1)
CHANGELOG.md (1)

3-9: Consider documenting additional changes in the changelog.

The changelog correctly documents the ably-cocoa update and 404 crash fix. However, two significant changes are not mentioned:

  1. iOS deployment target raised from 10.0 to 12.0 (visible in example/ios/Podfile and project configuration)
  2. Firebase-related frameworks now embedded as transitive dependencies (visible in example/ios/Runner.xcodeproj/project.pbxproj)

Users upgrading to this version should be aware of the iOS 12.0 minimum requirement.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between aca05b9 and d5195f6.

⛔ Files ignored due to path filters (5)
  • example/ios/Podfile.lock is excluded by !**/*.lock
  • example/pubspec.lock is excluded by !**/*.lock
  • pubspec.lock is excluded by !**/*.lock
  • test_integration/ios/Podfile.lock is excluded by !**/*.lock
  • test_integration/pubspec.lock is excluded by !**/*.lock
📒 Files selected for processing (6)
  • CHANGELOG.md (1 hunks)
  • README.md (1 hunks)
  • example/ios/Podfile (1 hunks)
  • example/ios/Runner.xcodeproj/project.pbxproj (1 hunks)
  • ios/ably_flutter.podspec (1 hunks)
  • pubspec.yaml (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
  • GitHub Check: build
  • GitHub Check: android (24)
  • GitHub Check: android (29)
  • GitHub Check: ios
  • GitHub Check: check
  • GitHub Check: android
  • GitHub Check: ios
🔇 Additional comments (6)
pubspec.yaml (1)

3-3: Version bump looks correct.

The version update to 1.2.42 is consistent with the CHANGELOG and README documentation.

example/ios/Podfile (1)

2-2: iOS deployment target bumped to 12.0.

This aligns with the dependency update to ably-cocoa 1.2.53.

README.md (1)

57-57: Documentation updated correctly.

The installation instruction reflects the new version 1.2.42.

ios/ably_flutter.podspec (2)

21-21: Ably CocoaPod dependency updated correctly.

The update from 1.2.33 to 1.2.53 addresses the 404 crash issue documented in the CHANGELOG.


23-23: Based on codebase inspection, the review comment identifies a real configuration inconsistency, though it's more complex than stated:

Actual inconsistency found:

  • ios/ably_flutter.podspec line 23 sets s.ios.deployment_target = '10.0'
  • example/ios/Podfile line 2 sets platform :ios, '12.0'
  • However, example/ios/Podfile post_install hook overrides this to '10.0', creating internal contradiction in the example Podfile itself

The codebase indeed specifies Ably version 1.2.53 as a dependency (podspec line 22). The core issue is that iOS 10.0 deployment target may be incompatible with Ably 1.2.53, which could have a higher minimum requirement. Without access to the CocoaPods spec for Ably 1.2.53, the exact minimum iOS version cannot be verified from within the sandbox.

Please verify:

  1. Check Ably CocoaPod 1.2.53 minimum iOS deployment target by visiting CocoaPods.org or running pod spec cat Ably in your local environment
  2. If Ably 1.2.53 requires iOS 12.0+, update both ios/ably_flutter.podspec (line 23) and example/ios/Podfile (line 2 and post_install hook line) to use consistent iOS 12.0
  3. If Ably 1.2.53 supports iOS 10.0, update example/ios/Podfile line 2 to match the podspec's 10.0, as the post_install hook already sets it correctly
example/ios/Runner.xcodeproj/project.pbxproj (1)

223-265: Firebase frameworks added as transitive dependencies.

These frameworks (FirebaseCore, FirebaseCoreInternal, FirebaseInstallations, FirebaseMessaging, GoogleDataTransport, GoogleUtilities, FBLPromises, Toast, nanopb) are embedded by CocoaPods, likely as transitive dependencies of Ably 1.2.53. This is auto-generated and expected behavior.

@maratal maratal merged commit d64f09a into main Nov 11, 2025
9 checks passed
@maratal maratal deleted the release/1.2.42 branch November 11, 2025 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants