[Oztechan/CCC#4574] Enable string catalog symbols#4575
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enables string catalog symbol generation for the iOS project by setting the STRING_CATALOG_GENERATE_SYMBOLS build setting and updates the Xcode project to version 14.3 (2620). However, the PR also includes unrelated changes to the scene phase handling logic that introduce issues.
- Enables
STRING_CATALOG_GENERATE_SYMBOLSbuild setting in both Debug and Release configurations - Updates Xcode project version from 1430 (Xcode 14.3) to 2620 (Xcode 16.2)
- Modifies scene phase change handling and logging in Application.swift
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| ios/CCC.xcodeproj/project.pbxproj | Adds STRING_CATALOG_GENERATE_SYMBOLS=YES to Debug and Release build configurations; updates LastUpgradeCheck to 2620 |
| ios/CCC.xcodeproj/xcshareddata/xcschemes/CCC.xcscheme | Updates LastUpgradeVersion to 2620 to match the project upgrade |
| ios/CCC/Application.swift | Modifies logging message and removes conditional check for onAppBackground event handling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ios/CCC/Application.swift
Outdated
|
|
||
| private func onScenePhaseChange(phase: ScenePhase) { | ||
| logger.i(message: { "Application onChange scenePhase \(phase)" }) | ||
| logger.i(message: { "Application onChange scenePhase ground \(phase)" }) |
There was a problem hiding this comment.
The word "ground" appears to be a typo in this log message. It seems like it should either be removed or replaced with a meaningful word. The message likely intended to say "Application onChange scenePhase (phase)" without "ground".
| logger.i(message: { "Application onChange scenePhase ground \(phase)" }) | |
| logger.i(message: { "Application onChange scenePhase \(phase)" }) |
| if scenePhase == .active { | ||
| observable.event.onAppBackground() | ||
| } | ||
| observable.event.onAppBackground() |
There was a problem hiding this comment.
Removing the conditional check that prevents onAppBackground from being called when coming from the background state may cause incorrect behavior. In iOS, the scene phase transitions through .inactive when going both to and from .background. The removed condition ensured that onAppBackground was only called when transitioning from .active to .inactive (user putting app in background), not when transitioning from .background to .inactive to .active (user returning to app). Without this check, onAppBackground will be called twice: once when the user backgrounds the app (active -> inactive) and again when they return (background -> inactive -> active). This could lead to duplicate background event handling.
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #4575 +/- ##
========================================
Coverage 55.63% 55.63%
========================================
Files 169 169
Lines 3013 3013
Branches 445 445
========================================
Hits 1676 1676
Misses 1312 1312
Partials 25 25 🚀 New features to boost your workflow:
|
mustafaozhan
force-pushed
the
4574-Enable-string-catalog-symbols
branch
from
a255179 to
8186f3b
Compare
|
1 participant
Resolves #4574