[CI] Implement detailed size metrics #3839
Conversation
WalkthroughAdds CI steps and Fastlane support for a detailed SDK size analysis: workflow runs general and detailed size metrics; Fastlane gains a Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant GH as GitHub Actions (sdk_size job)
participant FL as Fastlane
participant XC as Xcode Build
participant PL as Plugins (stream_actions, xcsize)
GH->>FL: Run General SDK Size Metrics\n(show_frameworks_sizes)
FL->>PL: invoke show_frameworks_sizes
PL-->>GH: General framework size summary
GH->>FL: Run Detailed SDK Size Metrics\n(size_analyze)
FL->>XC: gym build (Release, skip archive/package/codesign)
rect rgb(221,235,247)
note right of XC: Build configs set\nLD_GENERATE_MAP_FILE=YES\nLD_MAP_FILE_PATH -> linkmaps/...
end
XC-->>FL: Build artifacts + LinkMap files
FL->>PL: invoke show_detailed_sdk_size (xcsize)
PL-->>GH: Detailed size report (threshold-aware)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
fastlane/Fastfile (1)
935-946: LGTM! The size analysis lane is well-structured.The new
size_analyzelane correctly builds DemoApp for size analysis without unnecessary packaging or signing steps. The use ofshow_detailed_sdk_sizewith the sdk_names and threshold aligns with the PR's goal of implementing detailed size metrics.Optional: Consider adding derived_data_path for consistency.
For consistency with other lanes in this file (e.g., lines 315-332, 614-627), you might consider specifying
derived_data_pathandcloned_source_packages_pathin the gym call:lane :size_analyze do gym( scheme: 'DemoApp', configuration: 'Release', + derived_data_path: derived_data_path, + cloned_source_packages_path: source_packages_path, skip_archive: true, skip_package_ipa: true, skip_package_pkg: true, skip_codesigning: true ) show_detailed_sdk_size(sdk_names: sdk_names, threshold: 1) endThis would provide consistent build artifact locations across all lanes, though the current implementation should work correctly without them.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
Gemfile.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
.github/workflows/sdk-size-metrics.yml(2 hunks).gitignore(1 hunks)StreamChat.xcodeproj/project.pbxproj(6 hunks)fastlane/Fastfile(1 hunks)fastlane/Pluginfile(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). (3)
- GitHub Check: Build LLC + UI (Xcode 15)
- GitHub Check: Metrics
- GitHub Check: Metrics
🔇 Additional comments (2)
fastlane/Pluginfile (1)
8-9: LGTM! Plugin additions support the new size analysis.The version bump for
fastlane-plugin-stream_actionsand the addition offastlane-plugin-xcsizeappropriately support the detailed SDK size metrics feature introduced in this PR..gitignore (1)
102-102: LGTM! Appropriate exclusion of LinkMap build artifacts.The
*LinkMap.txtpattern correctly ignores linker map files that are generated as part of the detailed size analysis workflow.
| - name: Run Detailed SDK Size Metrics | ||
| run: bundle exec fastlane size_analyze | ||
| timeout-minutes: 30 |
There was a problem hiding this comment.
Restore secrets for the detailed lane.
size_analyze is expected to hit the same signing/App Store flows as show_frameworks_sizes. Without MATCH_PASSWORD and APPSTORE_API_KEY in this step’s env, the lane will fail in CI. Please mirror the secrets block from the general metrics step here.
🤖 Prompt for AI Agents
.github/workflows/sdk-size-metrics.yml around lines 39 to 41: the "Run Detailed
SDK Size Metrics" step runs fastlane size_analyze but does not restore the
required signing/App Store secrets, so CI will fail; copy/mirror the same
secrets/env block used by the general metrics step (add the MATCH_PASSWORD and
APPSTORE_API_KEY entries — or the entire env: secrets: block from that step)
into this step so size_analyze has the necessary credentials in CI.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
fastlane/Fastfile (2)
935-946: Consider adding cleanup before building.The
frameworks_sizesmethod (line 905) cleans up the build directory before starting. This lane may benefit from similar cleanup to ensure size metrics aren't affected by stale build artifacts, especially in CI environments where builds might be cached.If cleanup is needed, add this before the
gymcall:lane :size_analyze do # Cleanup previous builds FileUtils.rm_rf("../#{derived_data_path}/") gym( # ... existing options ) show_detailed_sdk_size(sdk_names: sdk_names, threshold: 42) endAlternatively, if reusing the build directory is intentional for performance, consider adding a comment documenting this behavior.
945-945: Document or make the threshold value configurable.The threshold value of 42 appears to be a magic number without explanation. Consider either:
- Making it configurable via
options[:threshold]with a default value- Adding a comment explaining what the threshold represents and why 42 was chosen
- Moving it to a constant at the top of the file
Option 1 - Make it configurable:
-lane :size_analyze do +lane :size_analyze do |options| gym( scheme: 'DemoApp', configuration: 'Release', skip_archive: true, skip_package_ipa: true, skip_package_pkg: true, skip_codesigning: true ) - show_detailed_sdk_size(sdk_names: sdk_names, threshold: 42) + show_detailed_sdk_size(sdk_names: sdk_names, threshold: options[:threshold] || 42) endOption 2 - Add a comment:
+ # Threshold in bytes: report objects with size changes >= 42 bytes show_detailed_sdk_size(sdk_names: sdk_names, threshold: 42)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
fastlane/Fastfile(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). (2)
- GitHub Check: Automated Code Review
- GitHub Check: Metrics
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
fastlane/Fastfile (1)
947-948: Previous review feedback addressed.The
derived_data_pathparameter requested in the previous review has been successfully added, ensuring consistent build cache behavior with other lanes.
🧹 Nitpick comments (1)
fastlane/Fastfile (1)
951-951: Consider documenting the threshold value.The threshold value of
42lacks context. Adding a comment explaining the unit (bytes? KB?) and purpose would improve maintainability.For example:
- show_detailed_sdk_size(sdk_names: sdk_names, threshold: 42) + # Report size changes >= 42 bytes + show_detailed_sdk_size(sdk_names: sdk_names, threshold: 42)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
fastlane/Fastfile(3 hunks)
🔇 Additional comments (1)
fastlane/Fastfile (1)
900-935: LGTM! Improved consistency and encapsulation.The changes enhance this lane in two ways:
- Changing from
deftoprivate_lanebetter encapsulates the internal implementation.- Adding
derived_data_pathandcloned_source_packages_pathensures consistent build caching with other lanes (e.g., lines 736, 758, 911).
SDK Size
|
StreamChat XCSizeNo changes in SDK size. |
StreamChatUI XCSizeNo changes in SDK size. |
|
3 participants
Resolve https://linear.app/stream/issue/IOS-1195
Summary by CodeRabbit