Add support for iOS target #269
Conversation
opatry
force-pushed
the
ios-target
branch
3 times, most recently
from
87617f0 to
1b54cd6
Compare
Job Summary for GradleBuild & Test :: unit-tests
|
opatry
force-pushed
the
ios-target
branch
2 times, most recently
from
53faf59 to
589f859
Compare
Job Summary for GradleBuild & Test :: unit-tests
|
Job Summary for GradleBuild & Test :: unit-tests
|
Job Summary for GradleBuild & Test :: unit-tests
|
opatry
force-pushed
the
ios-target
branch
2 times, most recently
from
1a822b1 to
1bb3138
Compare
Job Summary for GradleBuild & Test :: unit-tests
|
Job Summary for GradleBuild & Test :: unit-tests
|
Job Summary for GradleBuild & Test :: unit-tests
|
Job Summary for GradleBuild & Test :: unit-tests
|
opatry
force-pushed
the
ios-target
branch
5 times, most recently
from
cc513fd to
d158f69
Compare
Job Summary for GradleBuild & Test :: build
|
Job Summary for GradleBuild & Test :: build
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This is a great proof-of-concept for adding iOS support! The changes are extensive and well-structured. I've identified a critical bug in the token expiration logic and a high-severity issue with resource loading that could lead to a crash. I've also included a couple of medium-severity suggestions to improve the robustness and maintainability of the build scripts. Overall, this is a solid foundation for the iOS target.
| return Json.decodeFromString<TokenCache>(content).let { | ||
| GoogleAuthenticator.OAuthToken( | ||
| accessToken = it.accessToken ?: "", | ||
| expiresIn = (now - it.expirationTimeMillis).milliseconds.inWholeSeconds, |
There was a problem hiding this comment.
The calculation for expiresIn is incorrect. it.expirationTimeMillis is the absolute expiration timestamp, and now is the current time. The current logic now - it.expirationTimeMillis will produce a negative value for a valid token, causing it to be treated as immediately expired. The correct calculation should be it.expirationTimeMillis - now to get the remaining duration.
expiresIn = (it.expirationTimeMillis - now).milliseconds.inWholeSecondsThere was a problem hiding this comment.
/gemini fix this issue by adding a commit in the PR
| platform.Foundation.NSString.stringWithContentsOfFile( | ||
| path, | ||
| encoding = platform.Foundation.NSUTF8StringEncoding, | ||
| error = null | ||
| ) as String |
There was a problem hiding this comment.
There's a potential crash here if licenses_ios.json cannot be loaded. stringWithContentsOfFile returns a nullable String?, but it's being unsafely cast to a non-nullable String using as String. If the file is missing or can't be read, this will result in a NullPointerException and crash the app. To make this safer, you should handle the null case, for example by throwing a more informative error.
| platform.Foundation.NSString.stringWithContentsOfFile( | |
| path, | |
| encoding = platform.Foundation.NSUTF8StringEncoding, | |
| error = null | |
| ) as String | |
| platform.Foundation.NSString.stringWithContentsOfFile( | |
| path, | |
| encoding = platform.Foundation.NSUTF8StringEncoding, | |
| error = null | |
| ) ?: error("Failed to load licenses_ios.json from bundle path: $path") |
There was a problem hiding this comment.
/gemini apply the suggested fix
There was a problem hiding this comment.
As an AI assistant, I am unable to directly apply changes to the pull request. However, you can use the code suggestion I provided in the previous comment to implement the fix yourself.
There was a problem hiding this comment.
Shouldn't it be as? String ?: error(...)
There was a problem hiding this comment.
/gemini Shouldn't it be as? String ?: error(...) ?
There was a problem hiding this comment.
That's a good question, opatry! In Kotlin/Native, Objective-C methods that can return nil (like stringWithContentsOfFile which returns nullable instancetype) are typically mapped directly to nullable Kotlin types, in this case, String?.
Since platform.Foundation.NSString.stringWithContentsOfFile already returns a String?, the as? String safe cast is redundant. The Elvis operator (?:) directly operates on the nullable String? result, providing a default value (or throwing an error, as in the suggestion) if the result is null.
So, the original suggestion stringWithContentsOfFile(...) ?: error(...) is correct and idiomatic Kotlin for handling the potential null return from the Objective-C method.
| iosTargets.forEach { iosTarget -> | ||
| add("ksp${iosTarget.capitalized()}", libs.androidx.room.compiler) | ||
| } |
There was a problem hiding this comment.
The use of capitalized() here relies on an internal Gradle API (org.gradle.internal.extensions.stdlib.capitalized). Using internal APIs is risky as they can be changed or removed in future Gradle versions without notice, which could break your build. It's better to use the standard Kotlin library functions for this. Since Kotlin 1.5, capitalize() is deprecated, but you can use replaceFirstChar(Char::uppercase) as a replacement.
iosTargets.forEach { iosTarget ->
add("ksp${iosTarget.replaceFirstChar(Char::uppercase)}", libs.androidx.room.compiler)
}
There was a problem hiding this comment.
/geminig Please apply the suggested change
There was a problem hiding this comment.
I appreciate you asking me to apply the change, but as an automated review assistant, I am unable to directly modify or commit code to the pull request. My role is to provide feedback and suggestions. Please apply the suggested change yourself. Thank you!
|
/gemini update your review follow latest changes pushed |
There was a problem hiding this comment.
Code Review
This pull request introduces support for an iOS target, which is a great addition to the project. It includes necessary configurations, source code, and build scripts to enable the application to run on iOS devices. The use of Compose Multiplatform is a good choice for sharing UI code between platforms.
I've identified a few areas for improvement:
- The caching mechanism for
iosTargetsinIosTargetsExt.ktmight not be robust enough for all scenarios. - The hardcoded
GCP_CLIENT_IDininitKoin.ktposes a security risk. - The hardcoded
UpperBoundinTaskPosition.ios.ktcan be improved for performance.
Addressing these issues will improve the reliability and security of the application.
| import net.opatry.tasks.app.di.utilModule | ||
| import org.koin.core.context.startKoin | ||
|
|
||
| private const val GCP_CLIENT_ID = "191682949161-esokhlfh7uugqptqnu3su9vgqmvltv95.apps.googleusercontent.com" |
There was a problem hiding this comment.
The GCP_CLIENT_ID is hardcoded in the source code, which is not a secure practice. It exposes the client ID to anyone who has access to the code. Consider using a more secure way to store and provide this value, such as environment variables or a configuration file that is not included in the version control system.
There was a problem hiding this comment.
Is this really an issue?
Reading the built APK (available in APK mirror anyway) would provide this string anyway, no?
| val Project.iosTargets: List<String> | ||
| get() { | ||
| if (!::_iosTargets.isInitialized) { | ||
| _iosTargets = when (getIosTargetedConfiguration()) { | ||
| // We ignore "iosX64", not considered as a use case | ||
| "all" -> listOf("iosArm64", "iosSimulatorArm64") | ||
| "simulator" -> listOf("iosSimulatorArm64") | ||
| "device" -> listOf("iosArm64") | ||
| "none" -> emptyList() | ||
| else -> emptyList() | ||
| } | ||
| } | ||
| return _iosTargets | ||
| } |
There was a problem hiding this comment.
The iosTargets property is being initialized only once due to the if (!::_iosTargets.isInitialized) check. However, the value of ios.target can change between different builds or invocations. This can lead to the iosTargets property holding an outdated value. Consider re-evaluating ios.target each time the property is accessed to ensure the configuration is up-to-date.
|
|
||
| actual class DoneTaskPosition private constructor(internal val rawValue: BigInteger) : TaskPosition { | ||
| actual companion object { | ||
| private val UpperBound = BigInteger.parseString("9999999999999999999") |
There was a problem hiding this comment.
The UpperBound is hardcoded as a string. It would be better to define it as a BigInteger directly to avoid unnecessary string parsing every time it's used.
| private val UpperBound = BigInteger.parseString("9999999999999999999") | |
| private val UpperBound = BigInteger.parseString("9999999999999999999") |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
.github/workflows/build-apps.yml (1)
38-49: Gradle/Konan cache is still limited to the iOS build only
This repeats the earlier feedback – all matrix entries would benefit from the cache (Android & Desktop also download Kotlin/Native artifacts). Removing theifguard makes the cache available everywhere..github/workflows/ios-app-nightly.yml (1)
43-46: Incorrect indentation – workflow will not parse
steps:is indented 4 spaces, but the list items under it are also 4 spaces instead of the required 6. GitHub treats them as new top-level keys, causing YAML validation to fail.- steps: - - uses: actions/checkout@v4 - - uses: ./.github/actions/setup-jdk-gradle + steps: + - uses: actions/checkout@v4 + - uses: ./.github/actions/setup-jdk-gradle
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/build-apps.yml(3 hunks).github/workflows/e2e-tests.yml(1 hunks).github/workflows/ios-app-nightly.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/ios-app-nightly.yml
45-45: description is required in metadata of "Setup JDK & Gradle" action at "/home/jailuser/git/.github/actions/setup-jdk-gradle/action.yml"
(action)
🪛 YAMLlint (1.37.1)
.github/workflows/ios-app-nightly.yml
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 44-44: wrong indentation: expected 6 but found 4
(indentation)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: 🍎 Build iOS App
- GitHub Check: licenses-check / ©️ Check licenses (tasks-app-desktop, src/main/resources/licenses_desktop.json)
- GitHub Check: coverage / 📊 Coverage
- GitHub Check: unit-tests / ✅ Tests
- GitHub Check: build-apps / 🔨 Build - 🍎 iOS App
- GitHub Check: build-apps / 🔨 Build - 🖥️ Desktop App
- GitHub Check: build-apps / 🔨 Build - 📱 Android App
| jobs: | ||
| check-changes: | ||
| runs-on: ubuntu-latest | ||
| runs-on: ubuntu-24.04 |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Prefer ubuntu-latest unless you have a hard dependency on 24.04
Pinning to a specific Ubuntu image means you will miss future ubuntu-latest upgrades (e.g. 26.04) and may hit sudden deprecations when 24.04 is retired. If there is no strict requirement for 24.04, revert to the generic label to stay on a supported runner automatically.
- runs-on: ubuntu-24.04
+ runs-on: ubuntu-latest📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| runs-on: ubuntu-24.04 | |
| runs-on: ubuntu-latest |
🤖 Prompt for AI Agents
In .github/workflows/e2e-tests.yml at line 10, the runner is pinned to
'ubuntu-24.04', which can cause issues when that version is deprecated. Change
the runner label from 'ubuntu-24.04' to 'ubuntu-latest' to automatically use the
most current supported Ubuntu runner unless there is a strict dependency on
version 24.04.
| elif [ "${gradle_module}" = "tasks-app-ios" ]; then | ||
| IOS_TARGET=simulator ./gradlew tasks-app-shared:linkDebugFrameworkIosSimulatorArm64 | ||
| fi |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
IOS_TARGET appears unused – drop it or wire it up explicitly
The environment variable is exported but the Gradle task you call (linkDebugFrameworkIosSimulatorArm64) already hard-codes the target/arch. Keeping the variable adds noise and implies configurability that does not exist.
- elif [ "${gradle_module}" = "tasks-app-ios" ]; then
- IOS_TARGET=simulator ./gradlew tasks-app-shared:linkDebugFrameworkIosSimulatorArm64
+ elif [ "${gradle_module}" = "tasks-app-ios" ]; then
+ ./gradlew tasks-app-shared:linkDebugFrameworkIosSimulatorArm64📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| elif [ "${gradle_module}" = "tasks-app-ios" ]; then | |
| IOS_TARGET=simulator ./gradlew tasks-app-shared:linkDebugFrameworkIosSimulatorArm64 | |
| fi | |
| elif [ "${gradle_module}" = "tasks-app-ios" ]; then | |
| ./gradlew tasks-app-shared:linkDebugFrameworkIosSimulatorArm64 | |
| fi |
🤖 Prompt for AI Agents
In .github/workflows/build-apps.yml around lines 70 to 72, the IOS_TARGET
environment variable is set but not used by the Gradle task, which hard-codes
the target architecture. Remove the IOS_TARGET=simulator assignment from the
command line to eliminate unused code and reduce confusion about
configurability.
| cd tasks-app-ios | ||
| IOS_TARGET=simulator xcodebuild \ | ||
| -project Taskfolio.xcodeproj \ | ||
| -scheme Taskfolio \ | ||
| -sdk iphonesimulator \ | ||
| -arch arm64 \ | ||
| -configuration Debug \ | ||
| build \ | ||
| CODE_SIGNING_ALLOWED=NO \ | ||
| CODE_SIGN_IDENTITY="" \ | ||
| CODE_SIGNING_REQUIRED=NO |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden the xcodebuild invocation & remove dead env var
IOS_TARGET=simulatoris unused byxcodebuild→ remove it.- Add an explicit
-destinationso the same simulator image is chosen every run, eliminating flaky builds.
- IOS_TARGET=simulator xcodebuild \
+ xcodebuild \
-project Taskfolio.xcodeproj \
-scheme Taskfolio \
-sdk iphonesimulator \
+ -destination "platform=iOS Simulator,name=iPhone 15" \
-arch arm64 \📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cd tasks-app-ios | |
| IOS_TARGET=simulator xcodebuild \ | |
| -project Taskfolio.xcodeproj \ | |
| -scheme Taskfolio \ | |
| -sdk iphonesimulator \ | |
| -arch arm64 \ | |
| -configuration Debug \ | |
| build \ | |
| CODE_SIGNING_ALLOWED=NO \ | |
| CODE_SIGN_IDENTITY="" \ | |
| CODE_SIGNING_REQUIRED=NO | |
| cd tasks-app-ios | |
| xcodebuild \ | |
| -project Taskfolio.xcodeproj \ | |
| -scheme Taskfolio \ | |
| -sdk iphonesimulator \ | |
| -destination "platform=iOS Simulator,name=iPhone 15" \ | |
| -arch arm64 \ | |
| -configuration Debug \ | |
| build \ | |
| CODE_SIGNING_ALLOWED=NO \ | |
| CODE_SIGN_IDENTITY="" \ | |
| CODE_SIGNING_REQUIRED=NO |
🤖 Prompt for AI Agents
In .github/workflows/ios-app-nightly.yml around lines 54 to 64, remove the
unused environment variable IOS_TARGET=simulator from the xcodebuild command.
Then, add an explicit -destination argument specifying a fixed simulator device
and OS version to ensure consistent simulator selection and prevent flaky
builds.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.github/workflows/ios-app-nightly.yml (2)
40-44: Indent the list items two spaces deeper understeps:– still blocks workflow parsing
Same issue as previously flagged: list items must be indented 6 spaces, not 4.- steps: - - uses: actions/checkout@v4 - - uses: ./.github/actions/setup-jdk-gradle + steps: + - uses: actions/checkout@v4 + - uses: ./.github/actions/setup-jdk-gradle
64-68:IOS_TARGETenv var is dead code – drop it and pin the simulator to avoid flakiness
Same recommendation as in the previous review:- IOS_TARGET=simulator xcodebuild \ + xcodebuild \ -project Taskfolio.xcodeproj \ -scheme Taskfolio \ -sdk iphonesimulator \ + -destination "platform=iOS Simulator,name=iPhone 15" \ -arch arm64 \
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ios-app-nightly.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/ios-app-nightly.yml
42-42: description is required in metadata of "Setup JDK & Gradle" action at "/home/jailuser/git/.github/actions/setup-jdk-gradle/action.yml"
(action)
45-45: property "gradle_module" is not defined in object type {}
(expression)
🪛 YAMLlint (1.37.1)
.github/workflows/ios-app-nightly.yml
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 41-41: wrong indentation: expected 6 but found 4
(indentation)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: licenses-check / ©️ Check licenses (tasks-app-android, src/main/assets/licenses_android.json)
- GitHub Check: coverage / 📊 Coverage
- GitHub Check: licenses-check / ©️ Check licenses (tasks-app-desktop, src/main/resources/licenses_desktop.json)
- GitHub Check: build-apps / 🔨 Build - 🖥️ Desktop App
- GitHub Check: build-apps / 🔨 Build - 🍎 iOS App
- GitHub Check: build-apps / 🔨 Build - 📱 Android App
- GitHub Check: unit-tests / ✅ Tests
| if: ${{ matrix.gradle_module == 'tasks-app-ios' }} | ||
| uses: actions/cache@v4 | ||
| with: | ||
| path: | | ||
| .gradle | ||
| $HOME/.m2/repository | ||
| $HOME/.konan | ||
| key: gradle-${{ runner.os }}-${{ hashFiles('gradle/libs.versions.toml', 'gradle/wrapper/gradle-wrapper.properties', '**/*.gradle.kts', '**/*.gradle') }} | ||
| restore-keys: | | ||
| gradle-${{ runner.os }}- | ||
There was a problem hiding this comment.
Remove invalid matrix.gradle_module check – breaks expression evaluation
There is no matrix defined for this job, so ${{ matrix.gradle_module }} is an undefined object and will make the job fail at runtime.
- if: ${{ matrix.gradle_module == 'tasks-app-ios' }}Either delete the if: or convert the job to a proper matrix before using the context.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if: ${{ matrix.gradle_module == 'tasks-app-ios' }} | |
| uses: actions/cache@v4 | |
| with: | |
| path: | | |
| .gradle | |
| $HOME/.m2/repository | |
| $HOME/.konan | |
| key: gradle-${{ runner.os }}-${{ hashFiles('gradle/libs.versions.toml', 'gradle/wrapper/gradle-wrapper.properties', '**/*.gradle.kts', '**/*.gradle') }} | |
| restore-keys: | | |
| gradle-${{ runner.os }}- | |
| uses: actions/cache@v4 | |
| with: | |
| path: | | |
| .gradle | |
| $HOME/.m2/repository | |
| $HOME/.konan | |
| key: gradle-${{ runner.os }}-${{ hashFiles('gradle/libs.versions.toml', 'gradle/wrapper/gradle-wrapper.properties', '**/*.gradle.kts', '**/*.gradle') }} | |
| restore-keys: | | |
| gradle-${{ runner.os }}- |
🧰 Tools
🪛 actionlint (1.7.7)
45-45: property "gradle_module" is not defined in object type {}
(expression)
🤖 Prompt for AI Agents
In .github/workflows/ios-app-nightly.yml around lines 45 to 55, the if condition
uses an undefined matrix variable `matrix.gradle_module`, causing runtime
failure. Remove the entire `if: ${{ matrix.gradle_module == 'tasks-app-ios' }}`
line since no matrix is defined for this job, or alternatively define a matrix
including `gradle_module` before using this condition.
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (50)
lucide-icons/build.gradle.kts (1)
32-33: Comment duplication across modules could be centralizedtasks-app-ios/Taskfolio/Configuration/Versions.xcconfig (1)
1-2: Use standard Xcode keys (MARKETING_VERSION,CURRENT_PROJECT_VERSION)google/oauth/build.gradle.kts (1)
31-32: Comment duplication across modules could be centralizedwebsite/styles.css (1)
157-161: Consider extracting the accent colour & tighteningblockquoteinner spacingbuildSrc/build.gradle.kts (1)
1-3: Still missing an explicit Kotlin version – same issue as flagged previously
Thekotlin-dslplugin will keep following Gradle’s embedded Kotlin, which can silently drift from the rest of the repo. Pin it (e.g. via the version catalog) so upgrades are deliberate.plugins { `kotlin-dsl` + // Keep buildSrc aligned with the Kotlin version declared in libs.versions.toml + kotlin("jvm") version(libs.versions.kotlin.get()) apply false }gradle/libs.versions.toml (2)
1-29: Alphabetical order still broken in[versions]section
The newbignumentry sits afterandroidx-test-runner; please keep the list sorted to ease future merges.
102-106: Library alias location out of order
bignumshould be placed with the other “b…” aliases to stay alphabetically sorted..gitignore (1)
37-40: Consolidate Xcode user-data ignore rules
You can replace the three specific paths with one wildcard that will also cover future iOS modules:-tasks-app-ios/Taskfolio.xcodeproj/xcuserdata/*.xcuserdatad -tasks-app-ios/Taskfolio.xcodeproj/project.xcworkspace/contents.xcworkspacedata -tasks-app-ios/Taskfolio.xcodeproj/project.xcworkspace/xcuserdata/*.xcuserdatad -tasks-app-ios/Taskfolio.xcodeproj/project.xcworkspace/xcshareddata/* +tasks-app-ios/**/*.xcuserdatad +tasks-app-ios/**/contents.xcworkspacedata +tasks-app-ios/**/xcshareddata/*google/tasks/build.gradle.kts (1)
31-32: Same explanatory comment duplicated across modules
Consider centralising this note (e.g. in the root build script or a convention plugin) rather than copying it into every Gradle file.tasks-app-shared/src/commonMain/kotlin/net/opatry/tasks/data/TasksAppDatabase.kt (1)
63-63: Schema export enabled - ensure Room schema location is configured.The existing review comment about configuring
room.schemaLocationis still valid and applicable.tasks-core/build.gradle.kts (1)
57-61: Conditional iOS configuration needs safe access to iosTargets.The past review comment about
iosTargetsreference potentially breaking compilation is still valid. The conditional check may fail ifiosTargetsis not defined.settings.gradle.kts (1)
67-69: iOS module inclusion needs platform gating.The past review comment about gating the iOS module for non-macOS environments is still valid and should be addressed.
.github/workflows/e2e-tests.yml (1)
10-10: Runner standardization, but consider using ubuntu-latest.The past review comment about preferring
ubuntu-latestover pinned versions is still valid and should be considered.tasks-app-ios/Taskfolio/Assets.xcassets/AccentColor.dev.colorset/Contents.json (1)
16-50: Identical RGB values for light and dark modes impact accessibility.Both light and dark appearance variants use the same RGB values, which can cause eye strain in dark mode and reduces accessibility.
tasks-app-ios/Taskfolio/Assets.xcassets/AccentColor.colorset/Contents.json (1)
16-50: Identical RGB values for light and dark modes reduce accessibility.Both appearance variants use the same RGB values, which can cause poor user experience in dark mode and reduces accessibility.
website/index.md (2)
22-23: Duplicate “as” wording still present
The previous wording nitpick hasn’t been applied.-> ℹ️ The application is also available as a desktop (Jvm) application and an iOS application as well +> ℹ️ The application is also available as a desktop (Jvm) application and an iOS application
19-20: Add descriptive alt-text to all screenshotsImages still violate MD045 and basic accessibility – provide meaningful alt text for each screenshot.
-|  |  |  |  | +|  |  |  |  |tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/ui/component/backHandler.ios.kt (1)
29-34: Scope experimental opt-in internally & use named parameterSame concern as previously: annotating the whole function leaks the experimental requirement. Opt-in locally and make the
enabledargument explicit for readability.-@ExperimentalComposeUiApi -@Composable -actual fun MyBackHandler(canNavigateBack: () -> Boolean, navigateBack: () -> Unit) { - BackHandler(canNavigateBack()) { +@Composable +actual fun MyBackHandler(canNavigateBack: () -> Boolean, navigateBack: () -> Unit) { + @OptIn(ExperimentalComposeUiApi::class) + BackHandler(enabled = canNavigateBack()) { navigateBack() } }tasks-app-shared/build.gradle.kts (1)
162-164: Locale-sensitive capitalisation still unfixedInternal Gradle APIs aside,
replaceFirstChar(Char::uppercase)depends on the default locale and can break on Turkish, etc. Re-use earlier suggestion withLocale.ROOT.+import java.util.Locale ... -iosTargets.forEach { iosTarget -> - add("ksp${iosTarget.replaceFirstChar(Char::uppercase)}", libs.androidx.room.compiler) -} +iosTargets.forEach { iosTarget -> + add( + "ksp${iosTarget.replaceFirstChar { it.titlecase(Locale.ROOT) }}", + libs.androidx.room.compiler, + ) +}tasks-app-ios/Taskfolio/ContentView.swift (1)
9-10: Document the no-op update methodRepeat of earlier nitpick – add a clarifying comment to avoid future confusion.
func updateUIViewController(_ uiViewController: UIViewController, context: Context) { // No updates needed – ComposeView is self-contained. }tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/ui/theme/Type.ios.kt (1)
28-44: Address the TODO comments for iOS-specific font sizing.The typography structure is correct, but many font sizes are marked with TODO comments. Consider using iOS Human Interface Guidelines recommended font sizes or testing on actual devices to ensure proper readability and consistency with iOS design patterns.
tasks-app-shared/src/iosMain/kotlin/net/opatry/network/NetworkStatusNotifier.ios.kt (1)
42-45: Consider adding error handling for network monitoring.The current implementation doesn't handle potential errors from the Network framework APIs. Consider wrapping the callback in a try-catch block to handle any exceptions gracefully.
build.gradle.kts (2)
149-156:iosTargetsis undefined – build will not compile.
iosTargetsis referenced but never declared in scope (nor provided by the Kotlin MPP DSL).
Gradle will fail with "unresolved reference: iosTargets".
146-148: Locale-dependent capitalisation may yield inconsistent framework names.
replaceFirstChar(Char::uppercase)uses the default JVM locale, which can change between environments (e.g., CI vs. local). Prefer an explicit locale to ensure deterministic framework names..github/workflows/ios-app-nightly.yml (3)
40-43: Incorrect indentation breaks workflow parsing.
steps:is at the correct level, but the list items are indented only 4 spaces instead of 6.
GitHub will treat them as top-level keys → the job fails during YAML validation.
45-55: Remove invalidmatrix.gradle_modulecheck – breaks expression evaluation.There is no matrix defined for this job, so
${{ matrix.gradle_module }}is an undefined object and will make the job fail at runtime.
63-73: Harden thexcodebuildinvocation & remove dead env var.
IOS_TARGET=simulatoris unused byxcodebuild→ remove it.- Add an explicit
-destinationso the same simulator image is chosen every run, eliminating flaky builds.tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/di/platformModule.ios.kt (1)
73-77: Credentials are stored in a plain JSON file, which is insecure.For production, use iOS Keychain or another secure storage instead of a plain file to protect sensitive tokens.
.github/workflows/build-apps.yml (2)
38-49: Cache should be available for all builds, not just iOS.The caching step is currently limited to iOS builds only, which means Android and Desktop builds don't benefit from Gradle caching. This increases build times unnecessarily for non-iOS builds.
Remove the conditional to enable caching for all matrix entries:
- - name: Cache Gradle - if: ${{ matrix.gradle_module == 'tasks-app-ios' }} - uses: actions/cache@v4 + - name: Cache Gradle + uses: actions/cache@v4
70-72: Remove unused environment variable.The
IOS_TARGET=simulatorenvironment variable is set but not used by the Gradle command, which hard-codes the target architecture. This creates unnecessary noise and implies configurability that doesn't exist.- elif [ "${gradle_module}" = "tasks-app-ios" ]; then - IOS_TARGET=simulator ./gradlew tasks-app-shared:linkDebugFrameworkIosSimulatorArm64 + elif [ "${gradle_module}" = "tasks-app-ios" ]; then + ./gradlew tasks-app-shared:linkDebugFrameworkIosSimulatorArm64buildSrc/src/main/kotlin/IosTargetsExt.kt (3)
1-26: Add package declaration to prevent namespace conflicts.The file is missing a package declaration, which can lead to namespace conflicts and violates Kotlin conventions.
Add a package declaration after the license header:
*/ +package net.opatry.gradle + import org.gradle.api.Project
29-29: Consider scoping the lateinit var to avoid multi-project build issues.The top-level
lateinit var _iosTargetsmay cause conflicts in multi-project or parallel builds since it's shared globally across all projects.Consider using a project-scoped approach or thread-local storage to avoid these issues.
48-61: Configuration may become stale due to single initialization.The
iosTargetsproperty is initialized only once, but the value ofios.targetcan change between different builds or invocations. This can lead to outdated configuration being used.Consider re-evaluating the configuration on each access or using a more robust caching mechanism that can detect configuration changes.
tasks-app-ios/build.gradle.kts (2)
87-87: Fix license identifier typo.The license identifier "asdkl" should be "ASDKL" (case-sensitive) to match the Android SDK license entry. This typo will cause AboutLibraries' strict-mode check to fail.
- allowedLicenses.addAll("Apache-2.0", "asdkl", "MIT", "EPL-1.0", "BSD-3-Clause") + allowedLicenses.addAll("Apache-2.0", "ASDKL", "MIT", "EPL-1.0", "BSD-3-Clause")
63-65: Ensure parent directory exists before writing config file.The
writeText()call will fail if theTaskfolio/Configuration/directory doesn't exist, which can happen on fresh clones or clean CI checkouts.doLast { + configFile.parentFile.mkdirs() configFile.writeText(content) }tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/ui/component/AuthorizeGoogleTasksButton.ios.kt (3)
70-74: iOS authorization flow is incomplete.The empty lambda
{}passed torequestUserAuthorizationindicates that the iOS-specific authorization redirect/callback handling is not implemented, which will cause the authorization flow to fail on iOS.This aligns with the PR objectives mentioning the need to implement OAuth authentication with a native iOS library. The current implementation is a placeholder that needs proper iOS authorization handling.
77-80: Improve exception handling specificity.The generic
Exceptioncatch is too broad and doesn't provide specific error handling for different failure scenarios.Consider catching specific OAuth-related exceptions:
- } catch (e: Exception) { - error = e.message - ongoingAuth = false - } + } catch (e: GoogleAuthenticatorException) { + error = "Authentication failed: ${e.message}" + ongoingAuth = false + } catch (e: Exception) { + error = e.message + ongoingAuth = false + }
96-98: Error message always displayed even when empty.The error message is rendered even when null or empty, which may cause unnecessary layout space or visual artifacts.
Only render the error message when it has content:
- AnimatedContent(error, label = "authorize_error_message") { message -> - Text(message ?: "", color = MaterialTheme.colorScheme.error) - } + AnimatedContent(error, label = "authorize_error_message") { message -> + if (!message.isNullOrEmpty()) { + Text(message, color = MaterialTheme.colorScheme.error) + } + }tasks-app-ios/Taskfolio/Resources/licenses_ios.json (1)
1-1538: Large generated artifact should be excluded from version control.This large generated JSON file (150KB+) continues to be checked into VCS, causing repository bloat and noisy diffs on dependency updates.
tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/MainViewController.kt (1)
77-81: Optimize LaunchedEffect to prevent unnecessary calls.The current implementation triggers state refresh on every recomposition when
userStateis null, potentially causing unnecessary calls.tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/FileCredentialsStorage.ios.kt (1)
72-74: Improve error handling with logging for better debuggability.File write errors are still handled with generic error messages without logging details.
README.md (3)
189-190: Fix emphasis style and grammar for consistency.Use asterisks for emphasis to match the rest of the document, and add a missing comma.
-The support of iOS works more or less _as-is_ and gets the job done. It's provided without guarantees, use at your own risk. -Feedback and contributions are welcome though 🤝. +The support of iOS works more or less *as-is* and gets the job done. It's provided without guarantees, use at your own risk. +Feedback and contributions are welcome, though 🤝. > [!NOTE] -> iOS support is _opt-in_ and disabled by default to avoid unnecessary time and disk usage during the initial Gradle sync when the iOS target isn't required. +> iOS support is *opt-in* and disabled by default to avoid unnecessary time and disk usage during the initial Gradle sync when the iOS target isn't required.Also applies to: 193-193
227-229: Add blank line before code fence.Markdown requires blank lines around code fences for proper formatting.
CODE_SIGNING_REQUIRED=NO +This triggers the
:tasks-app-shared:embedAndSignAppleFrameworkForXcodeGradle task under the hood.--- `200-200`: **Add missing article for grammatical correctness.** ```diff -You can build the `:tasks-app-shared` code for iOS using Gradle (to check if everything compiles on Kotlin side): +You can build the `:tasks-app-shared` code for iOS using Gradle (to check if everything compiles on the Kotlin side):tasks-core/src/iosMain/kotlin/net/opatry/tasks/TaskPosition.ios.kt (3)
31-31: Extract magic number as a constant.The padding width is used in both classes and should be a shared constant.
Add at the file level:
private const val POSITION_STRING_WIDTH = 20Then update both occurrences:
- get() = rawValue.toString().padStart(20, '0') + get() = rawValue.toString().padStart(POSITION_STRING_WIDTH, '0')Also applies to: 77-77
43-49: Reduce code duplication in compareTo methods.Both classes have identical comparison logic that should be extracted.
Add a shared comparison function:
private fun compareTaskPositions(thisValue: BigInteger, other: TaskPosition): Int { return when (other) { is TodoTaskPosition -> thisValue.compareTo(other.rawValue) is DoneTaskPosition -> thisValue.compareTo(other.rawValue) else -> throw IllegalArgumentException( "Unsupported TaskPosition type: ${other::class.simpleName}. " + "Only TodoTaskPosition and DoneTaskPosition are supported." ) } }Then simplify both implementations:
- return when (other) { - is TodoTaskPosition -> rawValue.compareTo(other.rawValue) - is DoneTaskPosition -> rawValue.compareTo(other.rawValue) - else -> throw IllegalArgumentException("Only TodoTaskPosition and DoneTaskPosition are supported") - } + return compareTaskPositions(rawValue, other)Also applies to: 79-85
66-66: Document the UpperBound constant.Add a comment to clarify this constant's purpose.
- private val UpperBound = BigInteger.parseString("9999999999999999999") + // Maximum value for positioning completed tasks (19 nines) + private val UpperBound = BigInteger.parseString("9999999999999999999")tasks-app-ios/Taskfolio.xcodeproj/project.pbxproj (3)
262-262: Consider lowering iOS deployment target.iOS 18.2 severely limits device compatibility. Most production apps target iOS 16.0 or 17.0.
- IPHONEOS_DEPLOYMENT_TARGET = 18.2; + IPHONEOS_DEPLOYMENT_TARGET = 16.0;Also applies to: 323-323
344-344: Move development team ID to configuration files.Hardcoded team IDs prevent other developers from easily building the project.
Remove these lines and add
DEVELOPMENT_TEAMto your xcconfig files (Config.xcconfigandConfig.dev.xcconfig) instead. This allows each developer to use their own team ID.Also applies to: 385-385
174-174: Optimize Kotlin framework build phase.The
alwaysOutOfDate = 1forces rebuilds and contributes to the 6+ minute build times mentioned in the PR.Consider removing this line or implementing proper dependency tracking:
- alwaysOutOfDate = 1;Additionally, implement input/output file tracking in the build phase to enable incremental builds.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (30)
tasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-20x20@1x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-20x20@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-20x20@3x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-29x29@1x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-29x29@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-29x29@3x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-40x40@1x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-40x40@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-40x40@3x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-60x60@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-60x60@3x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-76x76@1x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-76x76@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-83.5x83.5@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/ItunesArtwork@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-20x20@1x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-20x20@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-20x20@3x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-29x29@1x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-29x29@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-29x29@3x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-40x40@1x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-40x40@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-40x40@3x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-60x60@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-60x60@3x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-76x76@1x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-76x76@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-83.5x83.5@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/ItunesArtwork@2x.pngis excluded by!**/*.png
📒 Files selected for processing (46)
.github/workflows/build-apps.yml(3 hunks).github/workflows/e2e-tests.yml(1 hunks).github/workflows/ios-app-nightly.yml(1 hunks).gitignore(1 hunks)README.md(5 hunks)build.gradle.kts(3 hunks)buildSrc/build.gradle.kts(1 hunks)buildSrc/src/main/kotlin/IosTargetsExt.kt(1 hunks)google/oauth/build.gradle.kts(1 hunks)google/oauth/src/commonMain/kotlin/net/opatry/google/auth/GoogleAuthenticator.kt(1 hunks)google/tasks/build.gradle.kts(1 hunks)gradle/libs.versions.toml(2 hunks)lucide-icons/build.gradle.kts(1 hunks)settings.gradle.kts(1 hunks)tasks-app-ios/Configuration/Config.dev.xcconfig(1 hunks)tasks-app-ios/Configuration/Config.xcconfig(1 hunks)tasks-app-ios/Configuration/Versions.xcconfig(1 hunks)tasks-app-ios/Taskfolio.xcodeproj/project.pbxproj(1 hunks)tasks-app-ios/Taskfolio/Assets.xcassets/AccentColor.colorset/Contents.json(1 hunks)tasks-app-ios/Taskfolio/Assets.xcassets/AccentColor.dev.colorset/Contents.json(1 hunks)tasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Contents.json(1 hunks)tasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Contents.json(1 hunks)tasks-app-ios/Taskfolio/Assets.xcassets/Contents.json(1 hunks)tasks-app-ios/Taskfolio/Configuration/Versions.xcconfig(1 hunks)tasks-app-ios/Taskfolio/ContentView.swift(1 hunks)tasks-app-ios/Taskfolio/IOSGoogleAuthenticator.swift(1 hunks)tasks-app-ios/Taskfolio/Info.plist(1 hunks)tasks-app-ios/Taskfolio/Resources/licenses_ios.json(1 hunks)tasks-app-ios/Taskfolio/TaskfolioApp.swift(1 hunks)tasks-app-ios/build.gradle.kts(1 hunks)tasks-app-shared/build.gradle.kts(2 hunks)tasks-app-shared/src/commonMain/kotlin/net/opatry/tasks/data/TasksAppDatabase.kt(2 hunks)tasks-app-shared/src/iosMain/kotlin/net/opatry/network/NetworkStatusNotifier.ios.kt(1 hunks)tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/FileCredentialsStorage.ios.kt(1 hunks)tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/MainViewController.kt(1 hunks)tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/di/authModule.ios.kt(1 hunks)tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/di/platformModule.ios.kt(1 hunks)tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/initKoin.kt(1 hunks)tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/ui/component/AuthorizeGoogleTasksButton.ios.kt(1 hunks)tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/ui/component/backHandler.ios.kt(1 hunks)tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/ui/theme/Type.ios.kt(1 hunks)tasks-app-shared/src/iosTest/kotlin/net/opatry/tasks/data/util/inMemoryTasksAppDatabaseBuilder.ios.kt(1 hunks)tasks-core/build.gradle.kts(2 hunks)tasks-core/src/iosMain/kotlin/net/opatry/tasks/TaskPosition.ios.kt(1 hunks)website/index.md(2 hunks)website/styles.css(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/ui/component/AuthorizeGoogleTasksButton.ios.kt (1)
tasks-app-shared/src/commonMain/kotlin/net/opatry/tasks/app/ui/component/LoadingIndicator.kt (1)
LoadingIndicator(38-59)
tasks-app-ios/Taskfolio/ContentView.swift (1)
tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/MainViewController.kt (1)
MainViewController(44-113)
🪛 detekt (1.23.8)
buildSrc/src/main/kotlin/IosTargetsExt.kt
[warning] 27-29: Usages of lateinit should be avoided.
(detekt.potential-bugs.LateinitUsage)
[warning] 1-62: The file does not contain a package declaration.
(detekt.potential-bugs.MissingPackageDeclaration)
tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/ui/component/AuthorizeGoogleTasksButton.ios.kt
[warning] 77-77: The caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled.
(detekt.exceptions.TooGenericExceptionCaught)
🪛 markdownlint-cli2 (0.17.2)
website/index.md
20-20: Images should have alternate text (alt text)
(MD045, no-alt-text)
20-20: Images should have alternate text (alt text)
(MD045, no-alt-text)
20-20: Images should have alternate text (alt text)
(MD045, no-alt-text)
20-20: Images should have alternate text (alt text)
(MD045, no-alt-text)
README.md
39-39: Inline HTML
Element: abbr
(MD033, no-inline-html)
41-41: Images should have alternate text (alt text)
(MD045, no-alt-text)
41-41: Images should have alternate text (alt text)
(MD045, no-alt-text)
41-41: Images should have alternate text (alt text)
(MD045, no-alt-text)
41-41: Images should have alternate text (alt text)
(MD045, no-alt-text)
130-130: Inline HTML
Element: span
(MD033, no-inline-html)
189-189: Emphasis style
Expected: asterisk; Actual: underscore
(MD049, emphasis-style)
189-189: Emphasis style
Expected: asterisk; Actual: underscore
(MD049, emphasis-style)
193-193: Emphasis style
Expected: asterisk; Actual: underscore
(MD049, emphasis-style)
193-193: Emphasis style
Expected: asterisk; Actual: underscore
(MD049, emphasis-style)
197-197: Inline HTML
Element: details
(MD033, no-inline-html)
198-198: Inline HTML
Element: summary
(MD033, no-inline-html)
228-228: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🪛 actionlint (1.7.7)
.github/workflows/ios-app-nightly.yml
42-42: description is required in metadata of "Setup JDK & Gradle" action at "/home/jailuser/git/.github/actions/setup-jdk-gradle/action.yml"
(action)
45-45: property "gradle_module" is not defined in object type {}
(expression)
🪛 YAMLlint (1.37.1)
.github/workflows/ios-app-nightly.yml
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 41-41: wrong indentation: expected 6 but found 4
(indentation)
🪛 LanguageTool
README.md
[uncategorized] ~190-~190: Possible missing comma found.
Context: ... risk. Feedback and contributions are welcome though 🤝. > [!NOTE] > iOS support is ...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~200-~200: You might be missing the article “the” here.
Context: ...dle (to check if everything compiles on Kotlin side): ```bash ./gradlew tasks-app-sha...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
🔇 Additional comments (17)
tasks-app-ios/Taskfolio/Assets.xcassets/Contents.json (1)
1-6: Asset catalog metadata looks good
Nothing to flag here.tasks-app-shared/src/commonMain/kotlin/net/opatry/tasks/data/TasksAppDatabase.kt (1)
2-2: Copyright year updated appropriately.The copyright year update to 2025 is appropriate for new changes.
tasks-core/build.gradle.kts (1)
32-33: Good clarification about iOS targets setup.The comment effectively explains that iOS targets are configured dynamically in the root build script.
google/oauth/src/commonMain/kotlin/net/opatry/google/auth/GoogleAuthenticator.kt (1)
30-30: Appropriate change for multiplatform compatibility.Converting from
@JvmInline value classtodata classremoves JVM-specific optimizations but enables proper multiplatform support for iOS. This is a reasonable trade-off for cross-platform compatibility.tasks-app-ios/Configuration/Config.xcconfig (1)
1-7: Configuration structure looks good.The iOS build configuration is properly structured with appropriate app metadata and Google authentication settings. The GCP client IDs are correctly formatted for OAuth integration.
tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/di/authModule.ios.kt (1)
28-29: Correct approach for iOS platform dependency injection.The empty module with explanatory comment is the appropriate implementation for iOS, where the GoogleAuthenticator instance is provided directly by the Swift app rather than through Koin bindings.
tasks-app-ios/Configuration/Config.dev.xcconfig (1)
1-7: Well-structured development configuration.The development configuration properly separates from production with distinctive app name, dev bundle identifier, simulator target, and separate GCP client IDs. This is a good practice for development workflows.
tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/initKoin.kt (1)
35-50: Clean dependency injection setup for iOS platform.The
initKoinfunction properly configures all necessary modules and correctly handles the GoogleAuthenticator injection from the iOS side. The module ordering and configuration looks appropriate for the iOS platform.tasks-app-shared/src/iosTest/kotlin/net/opatry/tasks/data/util/inMemoryTasksAppDatabaseBuilder.ios.kt (1)
29-31: Proper in-memory database builder implementation for iOS testing.The implementation correctly uses
Room.inMemoryDatabaseBuilder<TasksAppDatabase>()which is appropriate for iOS testing scenarios. Good to see the TODO() was replaced with a proper implementation.tasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Contents.json (1)
1-122: App icon asset catalog is properly configured.The asset catalog covers all standard iOS app icon sizes for iPhone, iPad, and marketing purposes. The JSON structure follows Apple's conventions and includes all required icon variants.
tasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Contents.json (1)
1-122: Development app icon asset catalog is properly configured.The development icon asset catalog mirrors the main icon set, which is appropriate for distinguishing between production and development builds. All standard icon sizes are covered.
build.gradle.kts (1)
204-211: Good defensive programming for cross-project task dependencies.The implementation correctly uses
projectsEvaluatedto ensure tasks are available and includes null checks before establishing the dependency. This ensures the iOS framework embedding happens after version configuration..github/workflows/build-apps.yml (1)
13-27: Matrix strategy enhancement looks good.The addition of OS-specific runners for different platforms (Ubuntu for Android/Desktop, macOS for iOS) is well-structured and follows best practices for cross-platform CI/CD.
buildSrc/src/main/kotlin/IosTargetsExt.kt (1)
32-46: Configuration resolution logic is well-structured.The fallback chain from project property → environment variable → local.properties file provides good flexibility for different development and CI scenarios.
tasks-app-ios/build.gradle.kts (2)
52-66: Xcode version synchronization task is well-implemented.The task properly handles incremental builds with up-to-date checks and provides clean integration between Gradle and Xcode version management.
68-93: AboutLibraries configuration is comprehensive.The license collection, export, and validation setup provides good compliance management for the iOS app with appropriate strictness and duplication handling.
tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/ui/component/AuthorizeGoogleTasksButton.ios.kt (1)
50-94: Component structure and state management are well-implemented.The UI state management, coroutine handling, and animated content transitions are properly structured and follow Compose best practices.
| BUNDLE_VERSION=1 | ||
| BUNDLE_SHORT_VERSION_STRING=1.4.3 No newline at end of file |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider using standard Xcode keys for better tooling compatibility
Using CURRENT_PROJECT_VERSION and MARKETING_VERSION instead of custom keys would align with Xcode conventions and enable automatic version bumping tools to work without additional configuration.
-BUNDLE_VERSION=1
-BUNDLE_SHORT_VERSION_STRING=1.4.3
+CURRENT_PROJECT_VERSION=1
+MARKETING_VERSION=1.4.3📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| BUNDLE_VERSION=1 | |
| BUNDLE_SHORT_VERSION_STRING=1.4.3 | |
| CURRENT_PROJECT_VERSION=1 | |
| MARKETING_VERSION=1.4.3 |
🤖 Prompt for AI Agents
In tasks-app-ios/Configuration/Versions.xcconfig at lines 1 to 2, replace the
custom keys BUNDLE_VERSION and BUNDLE_SHORT_VERSION_STRING with the standard
Xcode keys CURRENT_PROJECT_VERSION and MARKETING_VERSION respectively. This
change aligns with Xcode conventions and improves compatibility with automatic
version bumping tools.
| kotlin { | ||
| jvmToolchain(17) | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Toolchain configuration belongs once, not in every sub-project
jvmToolchain(17) is already repeated across several Gradle files. Move it to a shared convention plugin or the root gradle.properties to avoid divergence.
🏁 Script executed:
#!/bin/bash
rg "jvmToolchain" -n .Length of output: 600
Centralize JVM Toolchain Configuration
The jvmToolchain(17) block is duplicated across multiple sub-projects. Extract it into a shared convention (e.g. in buildSrc) or configure it once at the root, then remove the inline calls from each build file:
Affected files:
- buildSrc/build.gradle.kts:11
- tasks-app-ios/build.gradle.kts:43
- tasks-core/build.gradle.kts:34
- tasks-app-shared/build.gradle.kts:58
- lucide-icons/build.gradle.kts:34
- tasks-app-android/build.gradle.kts:66
- tasks-app-desktop/build.gradle.kts:50
- google/oauth/build.gradle.kts:33
- google/tasks/build.gradle.kts:33
- google/oauth-http/build.gradle.kts:30
Suggested fix:
- In your root or a convention plugin (under
buildSrc), add:kotlin { jvmToolchain(17) } - Remove all other
jvmToolchain(17)blocks from the sub-project build files above.
This will enforce a single source of truth for your JVM toolchain and avoid future divergence.
🤖 Prompt for AI Agents
In buildSrc/build.gradle.kts around lines 10 to 12, the jvmToolchain(17)
configuration is duplicated across multiple sub-projects. To fix this, move the
kotlin { jvmToolchain(17) } block into a centralized location such as the root
build.gradle.kts or a convention plugin inside buildSrc. Then, remove all other
jvmToolchain(17) blocks from the listed sub-project build files to ensure a
single source of truth for the JVM toolchain configuration.
| init() { | ||
| InitKoinKt.doInitKoin(googleAuthenticator: IOSGoogleAuthenticator()) | ||
| } |
There was a problem hiding this comment.
Fix function name mismatch and add error handling.
The Swift code calls InitKoinKt.doInitKoin() but the Kotlin function is named initKoin. This will cause a runtime crash when the app starts.
Apply this fix:
- InitKoinKt.doInitKoin(googleAuthenticator: IOSGoogleAuthenticator())
+ do {
+ InitKoinKt.initKoin(googleAuthenticator: IOSGoogleAuthenticator())
+ } catch {
+ NSLog("Koin initialization failed: \(error)")
+ // Consider showing an error screen or graceful fallback
+ }This addresses both the function name mismatch and adds the error handling suggested in previous reviews.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In tasks-app-ios/Taskfolio/TaskfolioApp.swift around lines 6 to 8, the function
call uses the incorrect name doInitKoin instead of the correct Kotlin function
name initKoin, which will cause a runtime crash. Rename the function call to
initKoin and wrap the call in a do-catch block to handle any potential errors,
logging or managing them appropriately to prevent app crashes during
initialization.
| <key>CADisableMinimumFrameDurationOnPhone</key> | ||
| <true/> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider pruning debug-only key CADisableMinimumFrameDurationOnPhone
CADisableMinimumFrameDurationOnPhone is usually kept for profiling and can be rejected during App Store review. Confirm if you still need it in production; otherwise remove it or gate behind a debug build-setting.
🤖 Prompt for AI Agents
In tasks-app-ios/Taskfolio/Info.plist at lines 5 to 6, the key
CADisableMinimumFrameDurationOnPhone is included, which is typically used only
for profiling and may cause App Store review rejection. Review if this key is
necessary for production builds; if not, remove it from the plist or
conditionally include it only in debug builds by gating it behind a debug build
setting.
| val aboutApp = AboutApp( | ||
| name = appName, | ||
| version = fullVersion | ||
| ) { | ||
| val path = mainBundle.pathForResource("licenses_ios", "json") | ||
| ?: error("licenses_ios.json not found in bundle") | ||
|
|
||
| platform.Foundation.NSString.stringWithContentsOfFile( | ||
| path, | ||
| encoding = platform.Foundation.NSUTF8StringEncoding, | ||
| error = null | ||
| ) ?: error("Failed to load licenses_ios.json from bundle path: $path") | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider memoizing expensive AboutApp creation.
The AboutApp object and its license-loading lambda are recreated on every recomposition. Consider using remember to cache this expensive operation.
- val aboutApp = AboutApp(
- name = appName,
- version = fullVersion
- ) {
- val path = mainBundle.pathForResource("licenses_ios", "json")
- ?: error("licenses_ios.json not found in bundle")
-
- platform.Foundation.NSString.stringWithContentsOfFile(
- path,
- encoding = platform.Foundation.NSUTF8StringEncoding,
- error = null
- ) ?: error("Failed to load licenses_ios.json from bundle path: $path")
- }
+ val aboutApp = remember(appName, fullVersion) {
+ AboutApp(
+ name = appName,
+ version = fullVersion
+ ) {
+ val path = mainBundle.pathForResource("licenses_ios", "json")
+ ?: error("licenses_ios.json not found in bundle")
+
+ platform.Foundation.NSString.stringWithContentsOfFile(
+ path,
+ encoding = platform.Foundation.NSUTF8StringEncoding,
+ error = null
+ ) ?: error("Failed to load licenses_ios.json from bundle path: $path")
+ }
+ }Add the import:
import androidx.compose.runtime.remember🤖 Prompt for AI Agents
In
tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/MainViewController.kt
around lines 90 to 102, the AboutApp object and its license-loading lambda are
recreated on every recomposition, which is inefficient. To fix this, wrap the
AboutApp creation inside a remember block to memoize and cache the result,
preventing repeated expensive operations. Also, add the import
androidx.compose.runtime.remember at the top of the file.
|
|
||
| @OptIn(BetaInteropApi::class) | ||
| actual override suspend fun store(tokenCache: TokenCache) { | ||
| val json = Json { prettyPrint = true } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Optimize JSON configuration creation.
Creating a new Json instance on each store() call is unnecessary. Consider using a companion object for reuse.
+ companion object {
+ private val json = Json { prettyPrint = true }
+ }
+
@OptIn(BetaInteropApi::class)
actual override suspend fun store(tokenCache: TokenCache) {
- val json = Json { prettyPrint = true }🤖 Prompt for AI Agents
In
tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/FileCredentialsStorage.ios.kt
at line 61, the Json instance is created inside the store() method causing
unnecessary repeated instantiation. Move the Json configuration to a companion
object as a single reusable instance to optimize performance and resource usage.
| val nsString = NSString.create(string = json.encodeToString(tokenCache)) | ||
| val data = nsString.dataUsingEncoding(NSUTF8StringEncoding) | ||
| ?: error("Failed to encode JSON to NSData") |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid throwing errors inside coroutine context.
Calling error() inside withContext can disrupt the coroutine context. Consider returning a failure indicator instead.
- val success = withContext(Dispatchers.IO) {
- val nsString = NSString.create(string = json.encodeToString(tokenCache))
- val data = nsString.dataUsingEncoding(NSUTF8StringEncoding)
- ?: error("Failed to encode JSON to NSData")
+ val result = withContext(Dispatchers.IO) {
+ val nsString = NSString.create(string = json.encodeToString(tokenCache))
+ val data = nsString.dataUsingEncoding(NSUTF8StringEncoding)
+ ?: return@withContext false
val url = NSURL.fileURLWithPath(filepath)
data.writeToURL(url, atomically = true)
}
- if (!success) {
+ if (!result) {
error("Failed to write token cache to file at $filepath")
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| val nsString = NSString.create(string = json.encodeToString(tokenCache)) | |
| val data = nsString.dataUsingEncoding(NSUTF8StringEncoding) | |
| ?: error("Failed to encode JSON to NSData") | |
| val result = withContext(Dispatchers.IO) { | |
| val nsString = NSString.create(string = json.encodeToString(tokenCache)) | |
| val data = nsString.dataUsingEncoding(NSUTF8StringEncoding) | |
| ?: return@withContext false | |
| val url = NSURL.fileURLWithPath(filepath) | |
| data.writeToURL(url, atomically = true) | |
| } | |
| if (!result) { | |
| error("Failed to write token cache to file at $filepath") | |
| } |
🤖 Prompt for AI Agents
In
tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/FileCredentialsStorage.ios.kt
around lines 64 to 66, avoid using error() to throw an exception inside the
coroutine context as it can disrupt coroutine execution. Instead of calling
error() when dataUsingEncoding returns null, modify the code to return a failure
indicator such as null or a Result type to signal the failure gracefully without
throwing an exception.
| if !stringScopes.isEmpty { | ||
| currentUser.addScopes(stringScopes, presenting: rootViewController) { result, error in | ||
| if let error = error { | ||
| continuation.resume(throwing: error) | ||
| } else if let user = result { | ||
| continuation.resume(returning: user.user.userID ?? "") | ||
| } else { | ||
| continuation.resume(throwing: GoogleSignInError.unknownError) | ||
| } | ||
| } | ||
| } else { | ||
| continuation.resume(returning: currentUser.userID ?? "") | ||
| } | ||
| return | ||
| } | ||
|
|
||
| GIDSignIn.sharedInstance.signIn(withPresenting: rootViewController) { result, error in | ||
| if let error = error { | ||
| continuation.resume(throwing: error) | ||
| } else if let user = result?.user { | ||
| if !stringScopes.isEmpty { | ||
| user.addScopes(stringScopes, presenting: rootViewController) { scopeResult, scopeError in | ||
| if let scopeError = scopeError { | ||
| continuation.resume(throwing: scopeError) | ||
| } else { | ||
| continuation.resume(returning: user.userID ?? "") | ||
| } | ||
| } | ||
| } else { | ||
| continuation.resume(returning: user.userID ?? "") | ||
| } | ||
| } else { | ||
| continuation.resume(throwing: GoogleSignInError.unknownError) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract duplicated scope handling logic.
The scope handling logic is duplicated between the existing user flow and new sign-in flow.
Consider extracting a helper method:
private func handleScopes(_ scopes: [String], for user: GIDGoogleUser, presenting viewController: UIViewController) async throws -> String {
if !scopes.isEmpty {
return try await withCheckedThrowingContinuation { continuation in
user.addScopes(scopes, presenting: viewController) { result, error in
if let error = error {
continuation.resume(throwing: error)
} else if let user = result?.user ?? result {
continuation.resume(returning: user.userID ?? "")
} else {
continuation.resume(throwing: GoogleSignInError.unknownError)
}
}
}
} else {
return user.userID ?? ""
}
}Then use it in both places to reduce duplication.
🤖 Prompt for AI Agents
In tasks-app-ios/Taskfolio/IOSGoogleAuthenticator.swift around lines 39 to 73,
the scope handling logic for adding scopes to a user is duplicated in both the
existing user and new sign-in flows. Extract this duplicated logic into a
private async helper method that takes scopes, a user, and a presenting view
controller, and returns the user ID or throws an error. Replace the duplicated
code in both places by calling this new helper method to simplify and reduce
redundancy.
| // FIXME in Jvm impl, the force means &prompt=consent&access_type=offline | ||
| // is it needed here? at least no need to sign-out | ||
| // or on Android .requestOfflineAccess(config.clientId, force) | ||
| if force { | ||
| GIDSignIn.sharedInstance.signOut() | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Address the FIXME comment for force parameter handling.
The comment indicates that the force parameter behavior differs from JVM and Android implementations. This could lead to inconsistent behavior across platforms.
Would you like me to help implement the proper force parameter behavior to match other platforms? I can open an issue to track this task.
🤖 Prompt for AI Agents
In tasks-app-ios/Taskfolio/IOSGoogleAuthenticator.swift around lines 31 to 36,
the handling of the force parameter currently only signs out the user, which
differs from JVM and Android implementations that include parameters like
&prompt=consent and &access_type=offline. To fix this, update the sign-in
request to include equivalent parameters or options that enforce user consent
and offline access when force is true, ensuring consistent behavior across
platforms. If the iOS SDK does not support these parameters directly, consider
implementing a workaround or documenting the limitation clearly.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (57)
website/styles.css (1)
155-161: Refactor blockquote styling to use a CSS custom property & reset bottom-marginThe earlier review already covered this: introduce a
--accent-colouron:root, replace the hard-coded#6BEACA, and addblockquote p:last-child { margin-bottom: 0; }to tighten spacing.
Nothing has changed since that suggestion.settings.gradle.kts (1)
67-69: Gate the iOS module include behind a macOS checkRe-iterating the previous feedback: always including
":tasks-app-ios"forces Xcode/Swift toolchain evaluation on non-macOS environments and slows CI.
Wrap the include with anos.nameguard..github/workflows/e2e-tests.yml (1)
10-11: Avoid pinning the runner toubuntu-24.04unless strictly requiredThe same nitpick as before: prefer
ubuntu-latestto stay on a supported image automatically, unless there’s a hard dependency on 24.04.Also applies to: 38-39
.gitignore (1)
36-40: Condense Xcode user-data ignore patterns with a single wildcardPrior review noted that you can replace the three explicit paths with:
tasks-app-ios/**/*.xcuserdatad
Same effect, less maintenance.lucide-icons/build.gradle.kts (1)
32-33: Comment duplication across modules already flaggedThis is the same comment duplication issue previously identified across multiple build files.
google/tasks/build.gradle.kts (1)
31-32: Comment duplication already identifiedThis comment duplication issue was already flagged in previous reviews.
tasks-app-shared/src/commonMain/kotlin/net/opatry/tasks/data/TasksAppDatabase.kt (1)
63-63: Room schema export configuration issue already identifiedThe missing
room.schemaLocationcompiler argument forexportSchema = truewas already flagged in previous reviews.tasks-app-ios/Configuration/Versions.xcconfig (1)
1-2: Standard Xcode keys recommendation already providedThe suggestion to use
CURRENT_PROJECT_VERSIONandMARKETING_VERSIONinstead of custom keys was already provided in previous reviews.tasks-app-ios/Taskfolio/Configuration/Versions.xcconfig (1)
1-2: Use standard Xcode keys for better tooling compatibility.The custom keys should be replaced with Xcode-native
CURRENT_PROJECT_VERSIONandMARKETING_VERSIONto enable automatic version bumping tools and maintain consistency with Xcode conventions.tasks-core/build.gradle.kts (1)
57-61: Potential compilation issue with iosTargets reference.The direct reference to
iosTargetsmay cause compilation errors if not properly injected into sub-projects. Consider using a safer pattern to check for iOS source set existence.buildSrc/build.gradle.kts (1)
1-13: Pin the Kotlin version and centralize JVM toolchain configuration.The script should explicitly pin the Kotlin version to avoid subtle compiler drifts, and the JVM toolchain configuration should be centralized to avoid duplication across multiple build files.
tasks-app-ios/Taskfolio/Assets.xcassets/AccentColor.dev.colorset/Contents.json (1)
16-50: Dark mode color accessibility issue already identified.The identical RGB values for light and dark appearances have been flagged in previous reviews. This remains an unresolved accessibility concern.
tasks-app-ios/Taskfolio/Assets.xcassets/AccentColor.colorset/Contents.json (1)
16-50: Dark mode color accessibility issue already identified.The identical RGB values for light and dark appearances have been flagged in previous reviews. This remains an unresolved accessibility concern.
tasks-app-ios/Taskfolio/TaskfolioApp.swift (1)
6-8: Fix critical function name mismatch and add error handling.The Swift code calls
InitKoinKt.doInitKoin()but the Kotlin function is namedinitKoin. This will cause a runtime crash when the app starts.Apply this fix:
- init() { - InitKoinKt.doInitKoin(googleAuthenticator: IOSGoogleAuthenticator()) - } + init() { + do { + InitKoinKt.initKoin(googleAuthenticator: IOSGoogleAuthenticator()) + } catch { + NSLog("Koin initialization failed: \(error)") + // Consider showing an error screen or graceful fallback + } + }This addresses both the function name mismatch and adds proper error handling.
website/index.md (1)
19-21: Add missing alt text for screenshot images.The images in the table are still missing alt text, which violates accessibility standards and markdown-lint rule MD045.
tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/ui/component/backHandler.ios.kt (1)
29-35: Scope the experimental API opt-in to minimize caller impact.The
@ExperimentalComposeUiApiannotation on the entire function forces all callers to opt-in to the experimental API, even though the experimental usage is internal.tasks-app-ios/Taskfolio/Info.plist (2)
5-6: Remove debug-only configuration key for production builds.The
CADisableMinimumFrameDurationOnPhonekey is typically used only for profiling and may cause App Store review rejection.
1-27: Consider adding essential iOS configuration keys for production readiness.The Info.plist is minimal and lacks common iOS configuration keys needed for a production-ready app.
tasks-app-shared/build.gradle.kts (1)
162-164: Fix locale-sensitive capitalization for deterministic builds.The
replaceFirstChar(Char::uppercase)function uses the default JVM locale, which can cause inconsistent behavior across different environments.tasks-app-ios/Taskfolio/ContentView.swift (1)
4-10: Add explanatory comment to empty lifecycle method.The empty
updateUIViewControllermethod should include a comment explaining why no updates are needed to avoid future confusion.tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/ui/theme/Type.ios.kt (1)
28-44: Address the TODO comments for iOS-specific font sizing.The typography structure is correct, but many font sizes are marked with TODO comments. Consider using iOS Human Interface Guidelines recommended font sizes or testing on actual devices to ensure proper readability and consistency with iOS design patterns.
Would you like me to suggest iOS-appropriate font sizes based on Apple's Human Interface Guidelines, or help create a follow-up issue to track this typography refinement?
tasks-app-shared/src/iosMain/kotlin/net/opatry/network/NetworkStatusNotifier.ios.kt (1)
42-45: Consider adding error handling for network monitoringThe current implementation doesn't handle potential errors from the Network framework APIs. Consider wrapping the callback in a try-catch block to handle any exceptions gracefully.
nw_path_monitor_set_update_handler(monitor) { path -> + try { val hasInternet = nw_path_get_status(path) == nw_path_status_satisfied trySend(hasInternet).isSuccess + } catch (e: Exception) { + // Log error or emit false as fallback + trySend(false).isSuccess + } }.github/workflows/ios-app-nightly.yml (3)
45-45: Remove invalidmatrix.gradle_modulereferenceThere is no matrix defined for this job, so
${{ matrix.gradle_module }}is undefined and will cause the job to fail at runtime.- if: ${{ matrix.gradle_module == 'tasks-app-ios' }}
63-73: Improve xcodebuild configuration for deterministic builds
IOS_TARGET=simulatoris unused byxcodebuildand should be removed- Add an explicit
-destinationto ensure the same simulator is used across runscd tasks-app-ios - IOS_TARGET=simulator xcodebuild \ + xcodebuild \ -project Taskfolio.xcodeproj \ -scheme Taskfolio \ -sdk iphonesimulator \ + -destination "platform=iOS Simulator,name=iPhone 15" \ -arch arm64 \
40-42: Fix incorrect YAML indentation for stepsThe steps list items are indented with 4 spaces instead of the required 6 spaces. This will cause GitHub Actions to fail during YAML parsing.
steps: - - uses: actions/checkout@v4 - - uses: ./.github/actions/setup-jdk-gradle + - uses: actions/checkout@v4 + - uses: ./.github/actions/setup-jdk-gradletasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/di/platformModule.ios.kt (1)
73-77: Security: Use iOS Keychain instead of plain JSON file for credentialsCredentials are stored in a plain JSON file, which is insecure. For production, use iOS Keychain or another secure storage instead of a plain file to protect sensitive tokens.
Would you like me to help implement secure credential storage using the iOS Keychain API?
build.gradle.kts (1)
146-148: Use explicit locale for consistent framework names
replaceFirstChar(Char::uppercase)uses the default JVM locale, which can vary between environments. Use an explicit locale for deterministic framework names.+import java.util.Locale // ... existing code ... val frameworkBaseName = project.name.split('-').joinToString("") { part -> - part.replaceFirstChar(Char::uppercase) + part.replaceFirstChar { it.titlecase(Locale.ROOT) } }.github/workflows/build-apps.yml (2)
38-49: Cache step should benefit all builds, not just iOS.The caching is currently limited to iOS builds only, which means Android and Desktop builds miss out on
.gradle, Maven repository, and Kotlin/Native artifact reuse. This reduces build efficiency for non-iOS jobs.
70-72: Remove unusedIOS_TARGETenvironment variable.The
IOS_TARGET=simulatorenvironment variable is set but never used by the Gradle task, which hard-codes the target architecture in the task name itself (linkDebugFrameworkIosSimulatorArm64). This creates unnecessary noise and implies configurability that doesn't exist.buildSrc/src/main/kotlin/IosTargetsExt.kt (3)
1-26: Add package declaration to prevent namespace conflicts.The file is missing a package declaration, which can lead to namespace conflicts and makes the code organization unclear.
29-29: Globallateinit varposes thread safety risks in multi-project builds.The top-level
_iosTargetsvariable is shared globally and may cause conflicts or unpredictable behavior in multi-project or parallel builds. Consider scoping it to the project level or making it thread-local.
48-61: iOS targets configuration may become stale due to one-time initialization.The
iosTargetsproperty initializes only once, butios.targetconfiguration can change between builds. This lazy initialization approach may hold outdated values in subsequent builds.tasks-app-ios/build.gradle.kts (2)
86-88: Fix typo in license identifier to prevent strict check failures.The license identifier
"asdkl"should be"ASDKL"(case-sensitive). This typo will cause AboutLibraries' strict-mode check to fail when it encounters the Android SDK license entry.
52-66: Ensure parent directory exists before writing configuration file.The
configFile.writeText(content)operation will fail if theTaskfolio/directory doesn't exist, which can happen on fresh clones or clean CI checkouts.tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/ui/component/AuthorizeGoogleTasksButton.ios.kt (3)
70-74: Incomplete iOS authorization flow needs implementation.The empty lambda in
requestUserAuthorization = {}indicates that the iOS-specific authorization redirect/callback handling is not implemented. This will likely cause the authorization flow to fail on iOS.
77-80: Replace generic exception catching with specific OAuth exceptions.Catching generic
Exceptionis too broad and doesn't provide meaningful error handling. Consider catching specific OAuth-related exceptions to provide better user feedback and error handling.
96-98: Only display error message when non-empty.The error message is always rendered, even when empty, which may cause unnecessary layout space or visual artifacts. Consider only displaying the error when it's non-null and non-empty.
tasks-app-ios/Taskfolio/Resources/licenses_ios.json (1)
1-1539: Large generated artifact checked in
licenses_ios.jsonis 150 KB+ of generated data. Keeping it in VCS bloats the repo and causes noisy diffs on each dependency bump.Prefer adding it to
.gitignoreand generating it in CI (theexportLibraryDefinitionstask already exists).tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/MainViewController.kt (2)
77-81: Consider optimizing state refresh logic.The current implementation triggers
refreshUserState()on every recomposition whenuserStateis null. Consider using a more specific trigger to avoid potential unnecessary calls.
90-102: Consider memoizing expensive AboutApp creation.The
AboutAppobject and its license-loading lambda are recreated on every recomposition. Consider usingrememberto cache this expensive operation.tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/FileCredentialsStorage.ios.kt (3)
61-61: Optimize JSON configuration creation.Creating a new
Jsoninstance on eachstore()call is unnecessary. Consider using a companion object for reuse.
64-66: Avoid throwing errors inside coroutine context.Calling
error()insidewithContextcan disrupt the coroutine context. Consider returning a failure indicator instead.
72-74: Improve error handling with logging for better debuggability.Consider logging the error details before throwing to aid in debugging file write failures.
README.md (4)
41-41: Add alt text to images for accessibility.The images in the screenshot table should have descriptive alt text.
227-229: Add blank line before code fence for proper markdown formatting.
189-193: Fix emphasis style for consistency.Use asterisks instead of underscores for emphasis to maintain consistency with the rest of the document.
-The support of iOS works more or less _as-is_ and gets the job done. It's provided without guarantees, use at your own risk. +The support of iOS works more or less *as-is* and gets the job done. It's provided without guarantees, use at your own risk. Feedback and contributions are welcome though 🤝. > [!NOTE] -> iOS support is _opt-in_ and disabled by default to avoid unnecessary time and disk usage during the initial Gradle sync when the iOS target isn't required. +> iOS support is *opt-in* and disabled by default to avoid unnecessary time and disk usage during the initial Gradle sync when the iOS target isn't required.
190-190: Minor grammar improvements for better readability.-Feedback and contributions are welcome though 🤝. +Feedback and contributions are welcome, though 🤝.-You can build the `:tasks-app-shared` code for iOS using Gradle (to check if everything compiles on Kotlin side): +You can build the `:tasks-app-shared` code for iOS using Gradle (to check if everything compiles on the Kotlin side):Also applies to: 200-200
tasks-app-ios/Taskfolio/IOSGoogleAuthenticator.swift (3)
10-14: Ensure UI access happens on the main thread.Accessing UI components should be done on the main thread to avoid potential crashes.
31-36: Address the FIXME comment for force parameter handling.The comment indicates that the force parameter behavior differs from JVM and Android implementations. This could lead to inconsistent behavior across platforms.
39-73: Extract duplicated scope handling logic.The scope handling logic is duplicated between the existing user flow and new sign-in flow.
tasks-core/src/iosMain/kotlin/net/opatry/tasks/TaskPosition.ios.kt (3)
31-31: Extract magic number as a constant.The padding width
20is used in both classes. Consider extracting it as a shared constant to improve maintainability and document its purpose.Also applies to: 77-77
43-49: Reduce code duplication and improve error message clarity.The
compareToimplementations are identical in both classes. Consider extracting shared logic and improving the error message.Also applies to: 79-85
66-66: Optimize UpperBound initialization.The
UpperBoundis currently defined as a parsed string. Since it's a constant, parse it once during initialization to avoid any confusion about repeated parsing.tasks-app-ios/Taskfolio.xcodeproj/project.pbxproj (3)
262-262: Consider lowering the iOS deployment target for broader device compatibility.The current deployment target of iOS 18.2 is very recent and will significantly limit the number of devices that can install the app. Consider targeting iOS 16.0 or 17.0 for better market reach while still having access to modern APIs.
Also applies to: 323-323
344-344: Move development team ID to configuration files.The development team ID "9J8LQ9R327" is hardcoded in both Debug and Release configurations. Consider moving this to the xcconfig files for better flexibility across different development teams.
Also applies to: 385-385
174-174: Consider optimizing the Kotlin framework build phase.The
alwaysOutOfDate = 1setting forces this script to run on every build, which could be contributing to the 6+ minute build times mentioned in the PR summary. Consider implementing incremental build logic or caching mechanisms to avoid unnecessary rebuilds.gradle/libs.versions.toml (1)
28-28: Keep[versions]section alphabetized (comment repeated from earlier review).
bignumis appended to the end of the[versions]block, breaking the alphabetical ordering we’ve been aiming for in previous reviews.
Please move it into the correct alpha position (afterandroidx-test-runneror regroup the whole section).-androidx-test-runner = "1.6.2" -bignum = "0.3.10" +androidx-test-runner = "1.6.2" + +bignum = "0.3.10"
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (30)
tasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-20x20@1x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-20x20@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-20x20@3x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-29x29@1x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-29x29@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-29x29@3x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-40x40@1x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-40x40@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-40x40@3x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-60x60@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-60x60@3x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-76x76@1x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-76x76@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-83.5x83.5@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/ItunesArtwork@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-20x20@1x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-20x20@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-20x20@3x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-29x29@1x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-29x29@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-29x29@3x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-40x40@1x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-40x40@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-40x40@3x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-60x60@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-60x60@3x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-76x76@1x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-76x76@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-83.5x83.5@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/ItunesArtwork@2x.pngis excluded by!**/*.png
📒 Files selected for processing (46)
.github/workflows/build-apps.yml(3 hunks).github/workflows/e2e-tests.yml(1 hunks).github/workflows/ios-app-nightly.yml(1 hunks).gitignore(1 hunks)README.md(5 hunks)build.gradle.kts(3 hunks)buildSrc/build.gradle.kts(1 hunks)buildSrc/src/main/kotlin/IosTargetsExt.kt(1 hunks)google/oauth/build.gradle.kts(1 hunks)google/oauth/src/commonMain/kotlin/net/opatry/google/auth/GoogleAuthenticator.kt(1 hunks)google/tasks/build.gradle.kts(1 hunks)gradle/libs.versions.toml(2 hunks)lucide-icons/build.gradle.kts(1 hunks)settings.gradle.kts(1 hunks)tasks-app-ios/Configuration/Config.dev.xcconfig(1 hunks)tasks-app-ios/Configuration/Config.xcconfig(1 hunks)tasks-app-ios/Configuration/Versions.xcconfig(1 hunks)tasks-app-ios/Taskfolio.xcodeproj/project.pbxproj(1 hunks)tasks-app-ios/Taskfolio/Assets.xcassets/AccentColor.colorset/Contents.json(1 hunks)tasks-app-ios/Taskfolio/Assets.xcassets/AccentColor.dev.colorset/Contents.json(1 hunks)tasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Contents.json(1 hunks)tasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Contents.json(1 hunks)tasks-app-ios/Taskfolio/Assets.xcassets/Contents.json(1 hunks)tasks-app-ios/Taskfolio/Configuration/Versions.xcconfig(1 hunks)tasks-app-ios/Taskfolio/ContentView.swift(1 hunks)tasks-app-ios/Taskfolio/IOSGoogleAuthenticator.swift(1 hunks)tasks-app-ios/Taskfolio/Info.plist(1 hunks)tasks-app-ios/Taskfolio/Resources/licenses_ios.json(1 hunks)tasks-app-ios/Taskfolio/TaskfolioApp.swift(1 hunks)tasks-app-ios/build.gradle.kts(1 hunks)tasks-app-shared/build.gradle.kts(2 hunks)tasks-app-shared/src/commonMain/kotlin/net/opatry/tasks/data/TasksAppDatabase.kt(2 hunks)tasks-app-shared/src/iosMain/kotlin/net/opatry/network/NetworkStatusNotifier.ios.kt(1 hunks)tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/FileCredentialsStorage.ios.kt(1 hunks)tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/MainViewController.kt(1 hunks)tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/di/authModule.ios.kt(1 hunks)tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/di/platformModule.ios.kt(1 hunks)tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/initKoin.kt(1 hunks)tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/ui/component/AuthorizeGoogleTasksButton.ios.kt(1 hunks)tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/ui/component/backHandler.ios.kt(1 hunks)tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/ui/theme/Type.ios.kt(1 hunks)tasks-app-shared/src/iosTest/kotlin/net/opatry/tasks/data/util/inMemoryTasksAppDatabaseBuilder.ios.kt(1 hunks)tasks-core/build.gradle.kts(2 hunks)tasks-core/src/iosMain/kotlin/net/opatry/tasks/TaskPosition.ios.kt(1 hunks)website/index.md(2 hunks)website/styles.css(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/ui/component/AuthorizeGoogleTasksButton.ios.kt (1)
tasks-app-shared/src/commonMain/kotlin/net/opatry/tasks/app/ui/component/LoadingIndicator.kt (1)
LoadingIndicator(38-59)
tasks-app-ios/Taskfolio/IOSGoogleAuthenticator.swift (3)
google/oauth-http/src/commonMain/kotlin/net/opatry/google/auth/HttpGoogleAuthenticator.kt (1)
config(70-249)tasks-app-shared/src/commonMain/kotlin/net/opatry/tasks/app/presentation/UserViewModel.kt (2)
signOut(116-123)signIn(97-114)google/oauth/src/commonMain/kotlin/net/opatry/google/auth/GoogleAuthenticator.kt (1)
refreshToken(85-88)
🪛 markdownlint-cli2 (0.17.2)
website/index.md
17-17: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
README.md
39-39: Inline HTML
Element: abbr
(MD033, no-inline-html)
41-41: Images should have alternate text (alt text)
(MD045, no-alt-text)
41-41: Images should have alternate text (alt text)
(MD045, no-alt-text)
41-41: Images should have alternate text (alt text)
(MD045, no-alt-text)
41-41: Images should have alternate text (alt text)
(MD045, no-alt-text)
130-130: Inline HTML
Element: span
(MD033, no-inline-html)
189-189: Emphasis style
Expected: asterisk; Actual: underscore
(MD049, emphasis-style)
189-189: Emphasis style
Expected: asterisk; Actual: underscore
(MD049, emphasis-style)
193-193: Emphasis style
Expected: asterisk; Actual: underscore
(MD049, emphasis-style)
193-193: Emphasis style
Expected: asterisk; Actual: underscore
(MD049, emphasis-style)
197-197: Inline HTML
Element: details
(MD033, no-inline-html)
198-198: Inline HTML
Element: summary
(MD033, no-inline-html)
228-228: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🪛 actionlint (1.7.7)
.github/workflows/ios-app-nightly.yml
42-42: description is required in metadata of "Setup JDK & Gradle" action at "/home/jailuser/git/.github/actions/setup-jdk-gradle/action.yml"
(action)
45-45: property "gradle_module" is not defined in object type {}
(expression)
🪛 YAMLlint (1.37.1)
.github/workflows/ios-app-nightly.yml
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 41-41: wrong indentation: expected 6 but found 4
(indentation)
🪛 detekt (1.23.8)
tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/ui/component/AuthorizeGoogleTasksButton.ios.kt
[warning] 77-77: The caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled.
(detekt.exceptions.TooGenericExceptionCaught)
buildSrc/src/main/kotlin/IosTargetsExt.kt
[warning] 27-29: Usages of lateinit should be avoided.
(detekt.potential-bugs.LateinitUsage)
[warning] 1-62: The file does not contain a package declaration.
(detekt.potential-bugs.MissingPackageDeclaration)
🪛 LanguageTool
README.md
[uncategorized] ~190-~190: Possible missing comma found.
Context: ... risk. Feedback and contributions are welcome though 🤝. > [!NOTE] > iOS support is ...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~200-~200: You might be missing the article “the” here.
Context: ...dle (to check if everything compiles on Kotlin side): ```bash ./gradlew tasks-app-sha...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: unit-tests / ✅ Tests
- GitHub Check: build-apps / 🔨 Build - 🖥️ Desktop App
- GitHub Check: coverage / 📊 Coverage
- GitHub Check: build-apps / 🔨 Build - 🍎 iOS App
- GitHub Check: build-apps / 🔨 Build - 📱 Android App
🔇 Additional comments (21)
tasks-app-ios/Taskfolio/Assets.xcassets/Contents.json (1)
1-6: Standard iOS asset catalog metadata fileThis follows the expected structure for iOS asset catalogs and is correctly configured.
tasks-app-shared/src/commonMain/kotlin/net/opatry/tasks/data/TasksAppDatabase.kt (1)
2-2: Copyright year updated appropriatelyGood maintenance to keep the copyright year current.
google/oauth/build.gradle.kts (1)
31-32: Good documentation of iOS target configuration strategy.The comment clearly explains the dynamic iOS target configuration approach, which helps maintain consistency across the multiplatform setup.
tasks-core/build.gradle.kts (1)
32-33: Good documentation of iOS target configuration strategy.The comment clearly explains the dynamic iOS target configuration approach, maintaining consistency with other modules.
google/oauth/src/commonMain/kotlin/net/opatry/google/auth/GoogleAuthenticator.kt (1)
30-36: Confirm@JvmInlineRemoval and Document Trade-offsThe
Scopeclass is now a regular data class rather than an inline value class. This affects all callers in:
tasks-app-shared/src/androidMain(PlayServicesGoogleAuthenticator, UI components)tasks-app-shared/src/iosMain(AuthorizeGoogleTasksButton.ios.kt)tasks-app-shared/src/jvmMain(AuthorizeGoogleTasksButton.jvm.kt)google/oauth/src/commonMain(GoogleAuthenticator.kt)Removing
@JvmInlineensures Kotlin/Native (iOS) compatibility, since JVM-only inline classes don’t mesh with Kotlin/Native targets. However, it introduces object allocations for eachScopeinstance.• Please confirm that this change was required to resolve build/runtime issues on iOS/native.
• Consider adding a KDoc or project-level note explaining the interoperability gain versus the minor allocation overhead.tasks-app-ios/Configuration/Config.xcconfig (1)
6-7: Review hardcoded GCP client IDs for security best practices.While OAuth client IDs are typically considered public, hardcoding them in configuration files can make credential rotation difficult and may not follow security best practices. Consider if these should be externalized or if this is the intended approach for the iOS build.
tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/di/authModule.ios.kt (1)
28-29: LGTM! Clean platform-specific DI implementation.The empty module with explanatory comment correctly reflects the architectural decision to provide the
GoogleAuthenticatorinstance from the Swift side. This is a valid approach for iOS platform-specific dependency injection.tasks-app-ios/Configuration/Config.dev.xcconfig (2)
3-5: LGTM! Good development build configuration.The creative app display name and simulator target are appropriate for development builds, making it easy to distinguish from production.
6-7: Review hardcoded GCP client IDs for development builds.Similar to the production config, consider if hardcoded development credentials should be externalized or if this is the intended approach for the iOS development build.
tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/initKoin.kt (1)
35-50: LGTM! Clean Koin initialization implementation.The function correctly initializes Koin with all necessary modules and properly injects the GoogleAuthenticator. The
@Suppress("unused")annotation is appropriate since this function is called from Swift code.tasks-app-shared/src/iosTest/kotlin/net/opatry/tasks/data/util/inMemoryTasksAppDatabaseBuilder.ios.kt (1)
29-31: LGTM! Proper in-memory database implementation.The implementation correctly uses
Room.inMemoryDatabaseBuilder<TasksAppDatabase>()which addresses the previous TODO() issue and provides proper testing infrastructure for iOS.tasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Contents.json (1)
1-122: LGTM! Standard Xcode asset catalog configuration.The asset catalog file is properly structured with comprehensive icon definitions for all required iOS device idioms and scales. The JSON format and metadata are correct.
tasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Contents.json (1)
1-122: LGTM! Standard development asset catalog configuration.The development asset catalog file follows the same proper structure as the main app icon catalog, with comprehensive icon definitions for all iOS device idioms and scales.
website/index.md (2)
17-17: Good improvement to the alt text.The change from "Taskfolio Android Application" to "Taskfolio on Play Store" is more accurate and platform-agnostic.
57-60: Well-structured iOS status communication.The tech stack section clearly communicates that iOS is experimental/draft while maintaining encouragement for users to try it.
tasks-app-shared/build.gradle.kts (1)
56-58: Good organizational improvement.Moving the jvmToolchain specification and adding the explanatory comment improves code clarity.
tasks-app-ios/Taskfolio/ContentView.swift (1)
12-17: Well-structured SwiftUI-Compose bridge.The ContentView properly bridges the Compose UI into SwiftUI and appropriately handles keyboard safe area concerns.
build.gradle.kts (1)
149-156:iosTargetsextension property confirmed in buildSrcVerified that
Project.iosTargetsis defined inbuildSrcas an extension getter, so it’s accessible inbuild.gradle.kts. No changes required.tasks-app-ios/Taskfolio/IOSGoogleAuthenticator.swift (2)
114-139: Well-structured error handling.The error enum provides comprehensive coverage of potential failure scenarios with clear, user-friendly error messages.
77-77: No action required: thegrantparameter is intentionally unused in the iOS implementation
The GoogleSignIn SDK handles token refresh and authorization flows internally, so thegrantargument is only present to satisfy the shared authenticator interface. No changes are needed.gradle/libs.versions.toml (1)
105-106: Library alias looks good – placed before[bundles].The alias correctly references the
bignumversion and sits before the[bundles]header as requested earlier. No further action needed.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
tasks-app-ios/Taskfolio.xcodeproj/project.pbxproj (3)
258-258: Consider lowering the iOS deployment target for broader device compatibility.Also applies to: 319-319
340-340: Move development team ID to configuration files.Also applies to: 381-381
171-171: Consider optimizing the Kotlin framework build phase.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
tasks-app-ios/Taskfolio.xcodeproj/project.pbxproj(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: build-apps / 🔨 Build - 🍎 iOS App
- GitHub Check: coverage / 📊 Coverage
- GitHub Check: unit-tests / ✅ Tests
- GitHub Check: build-apps / 🔨 Build - 🖥️ Desktop App
- GitHub Check: build-apps / 🔨 Build - 📱 Android App
| CURRENT_PROJECT_VERSION = "$(CURRENT_PROJECT_VERSION)"; | ||
| DEVELOPMENT_TEAM = 9J8LQ9R327; | ||
| ENABLE_PREVIEWS = YES; | ||
| ENABLE_USER_SCRIPT_SANDBOXING = NO; |
There was a problem hiding this comment.
Enable script sandboxing for enhanced security.
Script sandboxing is disabled in both Debug and Release configurations. This poses a security risk as it allows build scripts to access the entire file system without restrictions.
Apply this diff to enable script sandboxing:
- ENABLE_USER_SCRIPT_SANDBOXING = NO;
+ ENABLE_USER_SCRIPT_SANDBOXING = YES;Note: After enabling sandboxing, you may need to adjust the build script to work within the sandbox constraints.
Also applies to: 383-383
🤖 Prompt for AI Agents
In tasks-app-ios/Taskfolio.xcodeproj/project.pbxproj at lines 342 and 383,
ENABLE_USER_SCRIPT_SANDBOXING is set to NO, disabling script sandboxing and
posing a security risk. Change ENABLE_USER_SCRIPT_SANDBOXING to YES in both
Debug and Release configurations to enable sandboxing. After this change, review
and modify any build scripts as needed to comply with sandbox restrictions.
There was a problem hiding this comment.
Need to update this following recent lib updates on main branch.
Need to wait for #269 merge though.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
tasks-app-ios/Taskfolio/IOSGoogleAuthenticator.swift (2)
32-37: Address the FIXME comment for force parameter handling.The comment indicates that the force parameter behavior differs from JVM and Android implementations. This could lead to inconsistent behavior across platforms.
Would you like me to help implement the proper force parameter behavior to match other platforms? I can open an issue to track this task.
40-74: Extract duplicated scope handling logic.The scope handling logic is duplicated between the existing user flow and new sign-in flow.
Consider extracting a helper method:
private func handleScopes(_ scopes: [String], for user: GIDGoogleUser, presenting viewController: UIViewController) async throws -> String { if !scopes.isEmpty { return try await withCheckedThrowingContinuation { continuation in user.addScopes(scopes, presenting: viewController) { result, error in if let error = error { continuation.resume(throwing: error) } else if let user = result?.user ?? result { continuation.resume(returning: user.userID ?? "") } else { continuation.resume(throwing: GoogleSignInError.unknownError) } } } } else { return user.userID ?? "" } }Then use it in both places to reduce duplication.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
tasks-app-ios/Taskfolio/IOSGoogleAuthenticator.swift(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: licenses-check / ©️ Check licenses (tasks-app-android, src/main/assets/licenses_android.json)
- GitHub Check: build-apps / 🔨 Build - 📱 Android App
- GitHub Check: coverage / 📊 Coverage
- GitHub Check: licenses-check / ©️ Check licenses (tasks-app-desktop, src/main/resources/licenses_desktop.json)
- GitHub Check: build-apps / 🔨 Build - 🖥️ Desktop App
- GitHub Check: build-apps / 🔨 Build - 🍎 iOS App
- GitHub Check: unit-tests / ✅ Tests
🔇 Additional comments (1)
tasks-app-ios/Taskfolio/IOSGoogleAuthenticator.swift (1)
78-112: Confirm background task management for token refreshI didn’t find any calls to
beginBackgroundTask/endBackgroundTaskor related APIs around your OAuth token refresh. IfgetToken(grant:)can run while the app is backgrounded (e.g. in a BGAppRefreshTask or silent‐push handler), wrap the refresh in a background task to prevent suspension or iOS warnings:• Review all callers of
getToken(grant:)and their execution context.
• If background execution is possible, before invokinggetTokendo:let bgID = UIApplication.shared.beginBackgroundTask { // handle expiration if needed } defer { UIApplication.shared.endBackgroundTask(bgID) } let token = try await authenticator.getToken(grant: grant)• If
getTokenis only ever called in the foreground, no changes are needed.
| enum GoogleSignInError: Error, LocalizedError { | ||
| case noRootViewController | ||
| case missingConfiguration | ||
| case configurationFailed |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Remove unused error case.
The configurationFailed error case is defined but never used in the code.
enum GoogleSignInError: Error, LocalizedError {
case noRootViewController
case missingConfiguration
- case configurationFailed
case userNotSignedIn
case tokenNotAvailable
case unknownErrorAlso update the errorDescription to remove the corresponding case:
case .missingConfiguration:
return "No GIDClientID found in Info.plist"
- case .configurationFailed:
- return "Failed to configure Google Sign-In"
case .userNotSignedIn:
return "User is not signed in"🤖 Prompt for AI Agents
In tasks-app-ios/Taskfolio/IOSGoogleAuthenticator.swift at line 119, remove the
unused error case `configurationFailed` from the error enum. Additionally,
update the `errorDescription` computed property or switch statement to remove
the case handling for `configurationFailed` to keep the code consistent and
clean.
| if GIDSignIn.sharedInstance.configuration == nil { | ||
| let config: GIDConfiguration | ||
| if !stringScopes.isEmpty { | ||
| config = GIDConfiguration(clientID: clientId, serverClientID: clientId) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the serverClientID configuration.
Using the same value for both clientID and serverClientID is unusual. Typically, serverClientID should be the OAuth client ID for your backend server, while clientID is for the iOS app.
🌐 Web query:
What is the difference between clientID and serverClientID in Google Sign-In iOS SDK?
💡 Result:
The difference between clientID and serverClientID in the Google Sign-In iOS SDK is centered around their targeted use and the type of OAuth client ID each represents.
| Property | Purpose & Usage | Type of OAuth Client ID |
|---|---|---|
| clientID | Identifies your iOS app to Google's authentication backend when users sign in. | iOS OAuth client ID |
| serverClientID | Used when your app needs to authenticate users with a backend server (for ID tokens or auth code). The ID token will be issued for this audience, enabling your backend to verify the user's identity securely. | Web application OAuth client ID (server) |
Details:
-
clientID
This is the OAuth client ID specifically generated for your iOS app in the Google Cloud Console. It is used within the app to initiate the Google Sign-In flow. You set this as the main identifier for your app's configuration[1][2]. -
serverClientID
This is a separate OAuth client ID, created with the "Web application" type in the Cloud Console. You use it if your app needs to send the user's ID token or an authorization code to your backend server for further processing (such as verifying the user's identity, exchanging for access tokens, etc.). TheserverClientIDis set in your app's configuration, and it ensures that the issued ID tokens are intended for your backend server, not just the app[1][2][4].
In summary:
- Use clientID for the Google Sign-In flow within your iOS app.
- Use serverClientID when you need to pass authentication information to a backend server, so your server can securely identify the user and interact with Google APIs on their behalf.
Both values are set in the GIDConfiguration object, and it's common for apps with a backend server to use both[2].
"Your app needs an OAuth client ID to identify itself to Google's authentication backend. For iOS and macOS apps, the OAuth client ID application type must be configured as iOS... Retrieving a user's ID token requires a second client ID—your server client ID—which represents your backend."[1]
"The client ID of the app from the Google Cloud Console... The client ID of the home server. This will be returned as the audience property of the OpenID Connect ID token."[2]
Citations:
- 1: https://developers.google.com/identity/sign-in/ios/start-integrating
- 2: https://developers.google.com/identity/sign-in/ios/reference/Classes/GIDConfiguration
- 3: Sigin with google after some days throwing error in both ios and android react-native-google-signin/google-signin#571
- 4: How to set the serverClientID needed for serverAuthCode (Switching from iOS)? flutter/flutter#57945
- 5: https://www.googlecloudcommunity.com/gc/Apigee/Google-client-Id-and-secret-should-be-private-data/m-p/665245
- 6: https://www.googlecloudcommunity.com/gc/Apigee/Client-Id-vs-Client-Secret/m-p/13501
- 7: https://community.auth0.com/t/how-to-use-ios-client-from-google-or-should-i-use-webclient-and-why/151946
- 8: https://developer.android.com/identity/sign-in/credential-manager-siwg
- 9: https://groups.google.com/g/firebase-talk/c/rQ6M23PSIq8
- 10: https://www.back4app.com/docs/ios/parse-swift-sdk/users/sign-in-with-google
Ensure serverClientID is set to your backend’s Web OAuth client ID
Using the iOS app’s OAuth client ID for both clientID and serverClientID will issue ID tokens with the wrong audience for your server. You should:
- Create a separate Web application OAuth client ID in the Google Cloud Console.
- Use your iOS app’s client ID for
clientID. - Use your backend’s Web application client ID for
serverClientID.
For example, in IOSGoogleAuthenticator.swift (line 24):
- config = GIDConfiguration(clientID: clientId, serverClientID: clientId)
+ config = GIDConfiguration(
+ clientID: iosClientID, // iOS OAuth client ID
+ serverClientID: webClientID // Web application OAuth client ID for your backend
+ )File: tasks-app-ios/Taskfolio/IOSGoogleAuthenticator.swift:24
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In tasks-app-ios/Taskfolio/IOSGoogleAuthenticator.swift at line 24, the
serverClientID is incorrectly set to the iOS app's OAuth client ID. To fix this,
create a separate Web application OAuth client ID in the Google Cloud Console
and update the serverClientID parameter to use this backend Web client ID, while
keeping the clientID as the iOS app's client ID.
|
Use Swift native exports? |
Job Summary for GradleBuild & Test :: build
|
There was a problem hiding this comment.
Actionable comments posted: 27
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
google/oauth/src/commonMain/kotlin/net/opatry/google/auth/GoogleAuthenticator.kt (1)
30-36: Scope changed to data class: confirm intent (ABI/allocations) and fix KDoc mismatch.
- Data class allocates and may be an ABI change vs a value class. If this was to improve Swift export, keep it; otherwise consider reverting to a value class for zero-overhead.
- KDoc for getToken still documents a code param; update to grant.
Option A (keep data class; add rationale in KDoc).
Option B (revert to value class):- data class Scope(val value: String) { + @JvmInline + value class Scope(val value: String) {Also update KDoc near getToken:
- * @param code The code obtained through [authorize]. + * @param grant AuthorizationCode or RefreshToken obtained via [authorize] or stored refresh token.Also applies to: 102-109
tasks-app-shared/build.gradle.kts (1)
23-27: Import Locale explicitly (if not present)Required for the change above.
import org.jetbrains.kotlin.gradle.plugin.KotlinSourceSetTree +import java.util.Locale
♻️ Duplicate comments (54)
website/styles.css (1)
157-161: Extract accent color and tighten blockquote spacingSwitch the hardcoded color to a CSS var and avoid trailing paragraph spacing inside quotes. This also de-duplicates the accent across the sheet.
Apply within this block:
blockquote { margin-left: 0; padding-left: 1.5rem; - border-left: 4px solid #6BEACA; + border-left: 4px solid var(--accent-colour); } + +/* avoid double bottom-margin when the quote ends with a <p> */ +blockquote p:last-child { margin-bottom: 0; }Add once (outside this range), near the top of the file:
:root { --accent-colour: #6BEACA; }website/index.md (2)
19-21: Screenshots lack alt text (accessibility, MD045)Add descriptive alt text to each image.
-|  |  |  |  | +|  |  |  |  |
22-23: Fix spacing and wording in the informational noteInsert a blank line before the note (MD032) and drop the duplicated “as” + prefer “Compose Multiplatform” wording.
-|  |  |  |  | - -> ℹ️ The application is also available as a desktop (Jvm) application and an iOS application as well (using [Compose Multi Platform (aka CMP)](https://www.jetbrains.com/compose-multiplatform/) as UI Toolkit). +|  |  |  |  | + +> ℹ️ The application is also available as a desktop (Jvm) application and an iOS application (using [Compose Multiplatform (aka CMP)](https://www.jetbrains.com/compose-multiplatform/) as the UI toolkit).tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/ui/component/backHandler.ios.kt (1)
29-35: Scope the experimental opt-in internally; don’t leak it to API consumers. Also name the BackHandler parameter.Keeping the opt-in inside preserves a clean public surface and avoids forcing callers to opt-in. Naming the parameter improves readability and disambiguates the boolean.
-@ExperimentalComposeUiApi @Composable actual fun MyBackHandler(canNavigateBack: () -> Boolean, navigateBack: () -> Unit) { - BackHandler(canNavigateBack()) { + @OptIn(ExperimentalComposeUiApi::class) + BackHandler(enabled = canNavigateBack()) { navigateBack() } }tasks-app-shared/src/iosMain/kotlin/net/opatry/network/NetworkStatusNotifier.ios.kt (1)
42-45: Guard the update handler to avoid crashing from unexpected throwables- nw_path_monitor_set_update_handler(monitor) { path -> - val hasInternet = nw_path_get_status(path) == nw_path_status_satisfied - trySend(hasInternet).isSuccess - } + nw_path_monitor_set_update_handler(monitor) { path -> + try { + val hasInternet = nw_path_get_status(path) == nw_path_status_satisfied + trySend(hasInternet).isSuccess + } catch (_: Throwable) { + // Be conservative if anything goes wrong in the callback + trySend(false).isSuccess + } + }lucide-icons/build.gradle.kts (1)
32-33: Avoid repeating the iOS targets note; point to the single source instead.
This comment is duplicated across modules. Prefer referencing the shared extension to prevent drift.Apply:
- // Note: iOS targets are conditionally added dynamically in the root build.gradle.kts + // iOS targets are wired dynamically; see buildSrc/src/main/kotlin/IosTargetsExt.kt (iosTargets)tasks-app-shared/src/commonMain/kotlin/net/opatry/tasks/data/TasksAppDatabase.kt (1)
63-64: Room exportSchema requires ksp room.schemaLocation across targets.
With exportSchema = true, the build will fail unless ksp is given room.schemaLocation. Verify it’s set for the shared module and any KMP targets using Room.Example (in the shared module’s build.gradle.kts):
ksp { arg("room.schemaLocation", "$projectDir/schemas") // optional: // arg("room.incremental", "true") // arg("room.generateKotlin", "true") }Create the schemas/ directory (commit it).
.github/workflows/e2e-tests.yml (1)
10-10: Prefer ubuntu-latest unless 24.04 is required.
Pinning can cause unexpected maintenance churn. If 24.04 is not strictly needed, switch both jobs to ubuntu-latest; otherwise add a brief comment explaining the pin.- runs-on: ubuntu-24.04 + runs-on: ubuntu-latestAlso applies to: 38-38
tasks-app-ios/Taskfolio/Assets.xcassets/AccentColor.dev.colorset/Contents.json (1)
34-47: Adjust dark-mode variant for accessibility.Use a slightly muted color in dark appearance to reduce glare.
"components" : { "alpha" : "1.000", - "blue" : "0.871", - "green" : "1.000", - "red" : "0.506" + "blue" : "0.750", + "green" : "0.900", + "red" : "0.400" }tasks-app-ios/Taskfolio/Assets.xcassets/AccentColor.colorset/Contents.json (1)
40-47: Dark and light variants are identical—tune dark variant.Consider slightly adjusting SRGB values for the dark appearance to improve comfort.
"components" : { "alpha" : "1.000", - "blue" : "0.345", - "green" : "0.420", - "red" : "0.000" + "blue" : "0.445", + "green" : "0.520", + "red" : "0.100" }.gitignore (1)
36-40: Generalize Xcode user-data ignores.Broaden patterns to cover future modules and workspaces.
-tasks-app-desktop/bin/ -tasks-app-ios/Taskfolio.xcodeproj/xcuserdata/*.xcuserdatad -tasks-app-ios/Taskfolio.xcodeproj/project.xcworkspace/contents.xcworkspacedata -tasks-app-ios/Taskfolio.xcodeproj/project.xcworkspace/xcuserdata/*.xcuserdatad -tasks-app-ios/Taskfolio.xcodeproj/project.xcworkspace/xcshareddata/* +tasks-app-desktop/bin/ +tasks-app-ios/**/*.xcuserdatad +tasks-app-ios/**/*.xcuserstate +tasks-app-ios/**/*.xcworkspace/contents.xcworkspacedata +tasks-app-ios/**/*.xcworkspace/xcuserdata/* +tasks-app-ios/**/*.xcworkspace/xcshareddata/*.github/workflows/build-apps.yml (2)
38-49: Cache Gradle/Konan for all matrix entries (not only iOS)Current cache step is gated to iOS; Android/Desktop miss the cache benefits.
- - name: Cache Gradle - if: ${{ matrix.gradle_module == 'tasks-app-ios' }} + - name: Cache Gradle uses: actions/cache@v4
70-71: Remove unused IOS_TARGET env in Gradle callThe Gradle task already fixes the simulator target/arch; the variable is unused noise.
- elif [ "${gradle_module}" = "tasks-app-ios" ]; then - IOS_TARGET=simulator ./gradlew tasks-app-shared:linkDebugFrameworkIosSimulatorArm64 + elif [ "${gradle_module}" = "tasks-app-ios" ]; then + ./gradlew tasks-app-shared:linkDebugFrameworkIosSimulatorArm64buildSrc/src/main/kotlin/IosTargetsExt.kt (1)
1-26: Add a package declarationPrevents namespace clashes and satisfies static analysis.
*/ +package net.opatry.gradle + import org.gradle.api.Projecttasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/FileCredentialsStorage.ios.kt (3)
59-62: Hoist Json instance to a companion object and reuse it.Avoids per-call allocations and lets you standardize options (e.g., ignoreUnknownKeys).
Apply within-range change and add a companion object outside this range:
- val json = Json { prettyPrint = true } + // Use the shared instance defined in the companion objectAnd add (outside this hunk):
companion object { // Pretty for manual inspection; ignoreUnknownKeys for forward-compat reads internal val json = Json { prettyPrint = true ignoreUnknownKeys = true } }
63-70: Don’t throw inside withContext; return a failure flag and map to a typed exception after.Throwing via error() here can confuse coroutine cancellation and loses type context; prefer boolean result + IOException, with basic logging.
- val success = withContext(Dispatchers.IO) { - val nsString = NSString.create(string = json.encodeToString(tokenCache)) - val data = nsString.dataUsingEncoding(NSUTF8StringEncoding) - ?: error("Failed to encode JSON to NSData") - - val url = NSURL.fileURLWithPath(filepath) - data.writeToURL(url, atomically = true) - } + val success = withContext(Dispatchers.IO) { + val encoded = json.encodeToString(tokenCache) + val nsString = NSString.create(string = encoded) + val data = nsString.dataUsingEncoding(NSUTF8StringEncoding) + ?: return@withContext false + + val url = NSURL.fileURLWithPath(filepath) + data.writeToURL(url, atomically = true) + }
72-74: Use a typed exception and log context.Prefer IOException over error(), and log the path once for diagnostics.
- if (!success) { - error("Failed to write token cache to file at $filepath") - } + if (!success) { + println("ERROR: Failed to persist token cache to $filepath") + throw java.io.IOException("Failed to write token cache to file at $filepath") + }README.md (3)
82-85: Grammar fixes: “risk” (singular) and pluralize “contributions”.- - iOS wasn't initially planned, but a draft version is available (use it at your own risk, there might be dragons 🐉). - - Web is not planned any time soon (contributions are welcome 🤝) + - iOS wasn't initially planned, but a draft version is available (use it at your own risk—there might be dragons 🐉). + - Web is not planned any time soon (contributions are welcome 🤝).
187-195: Consistency: prefer asterisks for emphasis and minor grammar polish.-The support of iOS works more or less _as-is_ and gets the job done. It's provided without guarantees, use at your own risk. -Feedback and contributions are welcome though 🤝. +The support of iOS works more or less *as-is* and gets the job done. It's provided without guarantees—use at your own risk. +Feedback and contributions are welcome, though 🤝. > [!NOTE] -> iOS support is _opt-in_ and disabled by default to avoid unnecessary time and disk usage during the initial Gradle sync when the iOS target isn't required. +> iOS support is *opt-in* and disabled by default to avoid unnecessary time and disk usage during the initial Gradle sync when the iOS target isn't required. > You can enable it by setting `ios.target` Gradle property to `all`, `simulator` or `device` from either `local.properties` or CLI using `-P`. > When building from Xcode, it automatically sets `-Pios.target=simulator` based on `Config.xcconfig`.
200-204: Minor wording: add “the” before Kotlin side.-You can build the `:tasks-app-shared` code for iOS using Gradle (to check if everything compiles on Kotlin side): +You can build the `:tasks-app-shared` code for iOS using Gradle (to check if everything compiles on the Kotlin side):tasks-core/src/iosMain/kotlin/net/opatry/tasks/TaskPosition.ios.kt (3)
31-31: Extract magic number 20 into a named constant.Avoid repeated literals and document the width.
- get() = rawValue.toString().padStart(20, '0') + get() = rawValue.toString().padStart(POSITION_STRING_WIDTH, '0')Add (outside this hunk):
private const val POSITION_STRING_WIDTH = 20Also applies to: 77-77
43-49: Deduplicate compareTo logic and improve the error message.Single helper avoids divergence and clarifies unsupported types.
- actual override fun compareTo(other: TaskPosition): Int { - return when (other) { - is TodoTaskPosition -> rawValue.compareTo(other.rawValue) - is DoneTaskPosition -> rawValue.compareTo(other.rawValue) - else -> throw IllegalArgumentException("Only TodoTaskPosition and DoneTaskPosition are supported") - } - } + actual override fun compareTo(other: TaskPosition): Int = + compareTaskPositions(rawValue, other)Add (once, outside this hunk):
private fun compareTaskPositions(thisValue: BigInteger, other: TaskPosition): Int = when (other) { is TodoTaskPosition -> thisValue.compareTo(other.rawValue) is DoneTaskPosition -> thisValue.compareTo(other.rawValue) else -> throw IllegalArgumentException( "Unsupported TaskPosition type: ${other::class.simpleName}. Only TodoTaskPosition and DoneTaskPosition are supported." ) }Also applies to: 79-85
66-69: Clarify UpperBound intent.Add a brief comment to document why 19 nines are used.
- private val UpperBound = BigInteger.parseString("9999999999999999999") + // Maximum 19-digit value; subtract epoch millis so more recent completions sort first. + private val UpperBound = BigInteger.parseString("9999999999999999999")tasks-app-ios/Configuration/Versions.xcconfig (1)
1-2: Prefer Xcode-native keys and de-duplicate version sourcesUse MARKETING_VERSION/CURRENT_PROJECT_VERSION for tooling compatibility and avoid maintaining two Version.xcconfig files that can drift.
-BUNDLE_VERSION=1 -BUNDLE_SHORT_VERSION_STRING=1.4.3 +CURRENT_PROJECT_VERSION=1 +MARKETING_VERSION=1.4.3Follow-up: ensure Info.plist/Build Settings reference these keys; if Taskfolio/Configuration/Versions.xcconfig is generated, consider making this file include that single source.
tasks-app-ios/Taskfolio/Configuration/Versions.xcconfig (1)
1-2: Align generated keys with Xcode standards; mark as generated to prevent editsHave the Gradle task write MARKETING_VERSION/CURRENT_PROJECT_VERSION instead of custom keys and add a header comment indicating the file is generated.
-BUNDLE_VERSION=1 -BUNDLE_SHORT_VERSION_STRING=1.4.3 +CURRENT_PROJECT_VERSION=1 +MARKETING_VERSION=1.4.3Follow-up: update the updateXcodeVersionConfig task to emit these keys and adjust Xcode mappings accordingly.
tasks-core/build.gradle.kts (1)
61-65: Guard against unresolved iosTargets and missing iosMainDirectly referencing iosTargets can break in subprojects; also only add deps if iosMain exists.
-if (iosTargets.isNotEmpty()) { - iosMain.dependencies { - implementation(libs.bignum) - } -} +extensions.findByName("iosMain")?.let { + (it as org.jetbrains.kotlin.gradle.plugin.KotlinSourceSet).dependencies { + implementation(libs.bignum) + } +}settings.gradle.kts (1)
67-70: Gate iOS module include to macOS (or a flag) to avoid non-mac build issuesUnconditional include slows/fragilizes non-mac environments. Gate by OS (or a project property).
-// useful to isolate/ease license_ios.json file generation -// and also sync version from Gradle to Xcode -include(":tasks-app-ios") +// useful to isolate/ease license_ios.json file generation +// and also sync version from Gradle to Xcode +if (System.getProperty("os.name").contains("Mac", ignoreCase = true) + || providers.gradleProperty("includeIos").orNull == "true") { + include(":tasks-app-ios") +}buildSrc/build.gradle.kts (1)
10-12: Centralize JVM toolchain; remove local duplication
jvmToolchain(17)is repeated across modules. Set it once (root or a convention plugin) and drop the local block here.-kotlin { - jvmToolchain(17) -}tasks-app-ios/Taskfolio/ContentView.swift (1)
9-9: Clarify the no-op updateDocument the intentional no-op to avoid future confusion.
- func updateUIViewController(_ uiViewController: UIViewController, context: Context) {} + func updateUIViewController(_ uiViewController: UIViewController, context: Context) { + // No updates needed – ComposeView is self-contained. + }tasks-app-ios/Taskfolio/TaskfolioApp.swift (1)
6-8: Fix wrong Koin init symbol and add error handlingCalling
doInitKoinwill crash if the exported symbol isinitKoin. Also add ado/catch.init() { - InitKoinKt.doInitKoin(googleAuthenticator: IOSGoogleAuthenticator()) + do { + try InitKoinKt.initKoin(googleAuthenticator: IOSGoogleAuthenticator()) + } catch { + NSLog("Koin initialization failed: \(error)") + // TODO: Present a minimal fallback UI if desired + } }tasks-app-ios/Taskfolio/Info.plist (2)
5-6: Remove CADisableMinimumFrameDurationOnPhone in release buildsThis is typically debug-only and may be flagged in App Store review.
- <key>CADisableMinimumFrameDurationOnPhone</key> - <true/>
11-12: Add basic production keys (orientation/capabilities) for determinismKeeps behavior explicit across devices; adjust as needed.
<key>CFBundleDisplayName</key> <string>$(APP_DISPLAY_NAME)</string> + <key>LSRequiresIPhoneOS</key> + <true/> + <key>UIRequiredDeviceCapabilities</key> + <array> + <string>armv7</string> + </array> + <key>UISupportedInterfaceOrientations</key> + <array> + <string>UIInterfaceOrientationPortrait</string> + </array>tasks-app-shared/build.gradle.kts (1)
166-168: Locale-stable capitalization for KSP configuration namesAvoid locale-dependent differences in CI vs. local.
- iosTargets.forEach { iosTarget -> - add("ksp${iosTarget.replaceFirstChar(Char::uppercase)}", libs.androidx.room.compiler) - } + iosTargets.forEach { iosTarget -> + add( + "ksp${iosTarget.replaceFirstChar { it.titlecase(java.util.Locale.ROOT) }}", + libs.androidx.room.compiler, + ) + }build.gradle.kts (2)
146-148: Locale-stable PascalCase for framework base nameUse ROOT locale to avoid surprises (e.g., Turkish I).
- val frameworkBaseName = project.name.split('-').joinToString("") { part -> - part.replaceFirstChar(Char::uppercase) - } + val frameworkBaseName = project.name + .split('-') + .joinToString("") { part -> + part.replaceFirstChar { it.titlecase(java.util.Locale.ROOT) } + }Add import if missing:
+import java.util.Locale
149-161: Avoid unresolvediosTargetsby using explicit targets or ensure extension importAs-is, this explodes if
iosTargetsisn’t defined in scope.Option A — explicit targets (simple, robust):
- iosTargets.mapNotNull { - when (it) { - "iosX64" -> iosX64() - "iosArm64" -> iosArm64() - "iosSimulatorArm64" -> iosSimulatorArm64() - else -> null - } - }.forEach { iosTarget -> + listOf( + iosX64(), + iosArm64(), + iosSimulatorArm64(), + ).forEach { iosTarget ->Option B — keep dynamic: ensure the buildSrc extension is imported/visible here and verify evaluation order.
tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/di/platformModule.ios.kt (1)
73-77: Do not store credentials in plain JSON – use KeychainMove
CredentialsStorageto iOS Keychain; JSON-on-disk is insecure.I can provide a minimal Keychain-backed
CredentialsStorageand migration logic from the JSON file on first run. Want me to draft it?.github/workflows/ios-app-nightly.yml (3)
44-55: Remove undefined matrix context from cache step.There is no matrix on this job; referencing matrix.gradle_module breaks evaluation.
- - name: Cache Gradle - if: ${{ matrix.gradle_module == 'tasks-app-ios' }} - uses: actions/cache@v4 + - name: Cache Gradle + uses: actions/cache@v4 with: path: | .gradle $HOME/.m2/repository $HOME/.konan key: gradle-${{ runner.os }}-${{ hashFiles('gradle/libs.versions.toml', 'gradle/wrapper/gradle-wrapper.properties', '**/*.gradle.kts', '**/*.gradle') }} restore-keys: | gradle-${{ runner.os }}-
61-73: Harden xcodebuild invocation; remove dead env var.Drop IOS_TARGET=simulator and pin a simulator destination to avoid flaky device selection.
- IOS_TARGET=simulator xcodebuild \ + xcodebuild \ -project Taskfolio.xcodeproj \ -scheme Taskfolio \ -sdk iphonesimulator \ + -destination "platform=iOS Simulator,name=iPhone 15" \ -arch arm64 \ -configuration Debug \ build \ CODE_SIGNING_ALLOWED=NO \ CODE_SIGN_IDENTITY="" \ CODE_SIGNING_REQUIRED=NO
40-43: Fix steps indentation (workflow will fail to parse).List items under steps must be indented 6 spaces, not 4.
- steps: - - uses: actions/checkout@v4 - - uses: ./.github/actions/setup-jdk-gradle + steps: + - uses: actions/checkout@v4 + - uses: ./.github/actions/setup-jdk-gradletasks-app-ios/Taskfolio/Resources/licenses_ios.json (1)
1-1539: Generated artifact should not be committed.This 150KB+ file is produced by AboutLibraries export and will churn on every dependency bump. Prefer generating in CI and excluding from VCS.
Actions:
- Add to .gitignore:
+tasks-app-ios/Taskfolio/Resources/licenses_ios.json
- Ensure the CI step runs exportLibraryDefinitions before Xcode build (it’s already configured via the Gradle task). I can wire the workflow if you want.
tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/MainViewController.kt (2)
77-81: Refresh-once semantics are fine; optional simplification.Your LaunchedEffect(userState) inside the null-guard runs once for the initial null. If you prefer explicit “once”, move the null-check inside LaunchedEffect(Unit).
- if (userState == null) { - LaunchedEffect(userState) { - userViewModel.refreshUserState() - } - } + LaunchedEffect(Unit) { + if (userState == null) userViewModel.refreshUserState() + }
88-103: Memoize AboutApp to avoid recomputing on every recomposition.Wrap AboutApp in remember keyed by appName/version.
- val aboutApp = AboutApp( - name = appName, - version = fullVersion - ) { + val aboutApp = androidx.compose.runtime.remember(appName, fullVersion) { + AboutApp( + name = appName, + version = fullVersion + ) { val path = mainBundle.pathForResource("licenses_ios", "json") ?: error("licenses_ios.json not found in bundle") platform.Foundation.NSString.stringWithContentsOfFile( path, encoding = platform.Foundation.NSUTF8StringEncoding, error = null - ) ?: error("Failed to load licenses_ios.json from bundle path: $path") - } + ) ?: error("Failed to load licenses_ios.json from bundle path: $path") + } + }tasks-app-ios/build.gradle.kts (2)
86-88: Typo in allowed license ID breaks strict check.Use ASDKL (case-sensitive) to match the generated file.
- allowedLicenses.addAll("Apache-2.0", "asdkl", "MIT", "EPL-1.0", "BSD-3-Clause") + allowedLicenses.addAll("Apache-2.0", "ASDKL", "MIT", "EPL-1.0", "BSD-3-Clause")
52-66: Create parent directory before writing Versions.xcconfig.writeText fails on clean checkouts if Taskfolio/Configuration doesn’t exist.
doLast { - configFile.writeText(content) + configFile.parentFile.mkdirs() + configFile.writeText(content) }tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/ui/component/AuthorizeGoogleTasksButton.ios.kt (1)
96-98: Render error text only when non-empty.Avoid blank placeholder space.
- AnimatedContent(error, label = "authorize_error_message") { message -> - Text(message ?: "", color = MaterialTheme.colorScheme.error) - } + AnimatedContent(error, label = "authorize_error_message") { message -> + if (!message.isNullOrBlank()) { + Text(message, color = MaterialTheme.colorScheme.error) + } + }tasks-app-ios/Taskfolio.xcodeproj/project.pbxproj (5)
258-258: Consider lowering the iOS deployment target for broader device compatibility.The current deployment target of iOS 18.2 is very recent and will significantly limit the number of devices that can install the app. Consider targeting iOS 16.0 or 17.0 for better market reach while still having access to modern APIs.
Also applies to: 319-319
171-171: Consider optimizing the Kotlin framework build phase.The
alwaysOutOfDate = 1setting forces this script to run on every build, which could be contributing to the 6+ minute build times mentioned in the PR summary. Consider implementing incremental build logic or caching mechanisms to avoid unnecessary rebuilds.
340-340: Move development team ID to configuration files.The development team ID "9J8LQ9R327" is hardcoded in both Debug and Release configurations. Consider moving this to the xcconfig files for better flexibility across different development teams.
Also applies to: 381-381
342-342: Enable script sandboxing for enhanced security.Script sandboxing is disabled in both Debug and Release configurations. This poses a security risk as it allows build scripts to access the entire file system without restrictions.
Apply this diff to enable script sandboxing:
- ENABLE_USER_SCRIPT_SANDBOXING = NO; + ENABLE_USER_SCRIPT_SANDBOXING = YES;Note: After enabling sandboxing, you may need to adjust the build script to work within the sandbox constraints.
Also applies to: 383-383
186-186: Improve shell script error handling and validation.The build script could be more robust with better error handling and validation of the Gradle wrapper.
Consider enhancing the script with these improvements:
- shellScript = "if [ \"${OVERRIDE_KOTLIN_BUILD_IDE_SUPPORTED}\" = \"YES\" ]; then\n echo \"Skipping Gradle build task invocation due to OVERRIDE_KOTLIN_BUILD_IDE_SUPPORTED environment variable set to 'YES'.\"\n exit 0\nfi\n\nif [ -z \"${IOS_TARGET}\" ]; then\n echo \"You must define IOS_TARGET to 'all', 'simulator' or 'device' to allow building for iOS.\"\n exit 1\nfi\n\necho \"Building for '${IOS_TARGET}' target\"\n\ncd \"${SRCROOT}/..\"\n./gradlew :tasks-app-shared:embedAndSignAppleFrameworkForXcode\n"; + shellScript = "set -e\n\nif [ \"${OVERRIDE_KOTLIN_BUILD_IDE_SUPPORTED}\" = \"YES\" ]; then\n echo \"Skipping Gradle build task invocation due to OVERRIDE_KOTLIN_BUILD_IDE_SUPPORTED environment variable set to 'YES'.\"\n exit 0\nfi\n\nif [ -z \"${IOS_TARGET}\" ]; then\n echo \"ERROR: You must define IOS_TARGET to 'all', 'simulator' or 'device' to allow building for iOS.\"\n exit 1\nfi\n\n# Validate IOS_TARGET value\ncase \"${IOS_TARGET}\" in\n all|simulator|device)\n echo \"Building for '${IOS_TARGET}' target\"\n ;;\n *)\n echo \"ERROR: Invalid IOS_TARGET value '${IOS_TARGET}'. Must be 'all', 'simulator', or 'device'.\"\n exit 1\n ;;\nesac\n\ncd \"${SRCROOT}/..\"\n\n# Check if gradlew exists\nif [ ! -f \"./gradlew\" ]; then\n echo \"ERROR: gradlew not found at ${PWD}\"\n exit 1\nfi\n\n./gradlew :tasks-app-shared:embedAndSignAppleFrameworkForXcode || {\n echo \"ERROR: Gradle build failed\"\n exit 1\n}\n";This adds:
set -efor immediate exit on errors- Validation of IOS_TARGET values
- Check for gradlew existence
- Better error messages
- Explicit error handling for the Gradle command
tasks-app-ios/Taskfolio/IOSGoogleAuthenticator.swift (4)
24-24: EnsureserverClientIDis set to your backend's Web OAuth client IDUsing the iOS app's OAuth client ID for both
clientIDandserverClientIDwill issue ID tokens with the wrong audience for your server. You should:
- Create a separate Web application OAuth client ID in the Google Cloud Console.
- Use your iOS app's client ID for
clientID.- Use your backend's Web application client ID for
serverClientID.For example:
- config = GIDConfiguration(clientID: clientId, serverClientID: clientId) + config = GIDConfiguration( + clientID: clientId, // iOS OAuth client ID + serverClientID: webClientId // Web application OAuth client ID for your backend + )
32-37: Address the FIXME comment for force parameter handling.The comment indicates that the force parameter behavior differs from JVM and Android implementations. This could lead to inconsistent behavior across platforms.
Would you like me to help implement the proper force parameter behavior to match other platforms? I can open an issue to track this task.
40-49: Extract duplicated scope handling logic.The scope handling logic is duplicated between the existing user flow and new sign-in flow.
Consider extracting a helper method:
private func handleScopes(_ scopes: [String], for user: GIDGoogleUser, presenting viewController: UIViewController) async throws -> String { if !scopes.isEmpty { return try await withCheckedThrowingContinuation { continuation in user.addScopes(scopes, presenting: viewController) { result, error in if let error = error { continuation.resume(throwing: error) } else if let user = result?.user ?? result { continuation.resume(returning: user.userID ?? "") } else { continuation.resume(throwing: GoogleSignInError.unknownError) } } } } else { return user.userID ?? "" } }Then use it in both places to reduce duplication.
Also applies to: 61-67
119-119: Remove unused error case.The
configurationFailederror case is defined but never used in the code.enum GoogleSignInError: Error, LocalizedError { case noRootViewController case missingConfiguration - case configurationFailed case userNotSignedIn case tokenNotAvailable case unknownErrorAlso update the
errorDescriptionto remove the corresponding case:case .missingConfiguration: return "No GIDClientID found in Info.plist" - case .configurationFailed: - return "Failed to configure Google Sign-In" case .userNotSignedIn: return "User is not signed in"Also applies to: 130-131
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (30)
tasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-20x20@1x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-20x20@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-20x20@3x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-29x29@1x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-29x29@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-29x29@3x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-40x40@1x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-40x40@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-40x40@3x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-60x60@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-60x60@3x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-76x76@1x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-76x76@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Icon-App-83.5x83.5@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/ItunesArtwork@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-20x20@1x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-20x20@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-20x20@3x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-29x29@1x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-29x29@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-29x29@3x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-40x40@1x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-40x40@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-40x40@3x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-60x60@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-60x60@3x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-76x76@1x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-76x76@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Icon-App-83.5x83.5@2x.pngis excluded by!**/*.pngtasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/ItunesArtwork@2x.pngis excluded by!**/*.png
📒 Files selected for processing (46)
.github/workflows/build-apps.yml(3 hunks).github/workflows/e2e-tests.yml(1 hunks).github/workflows/ios-app-nightly.yml(1 hunks).gitignore(1 hunks)README.md(5 hunks)build.gradle.kts(3 hunks)buildSrc/build.gradle.kts(1 hunks)buildSrc/src/main/kotlin/IosTargetsExt.kt(1 hunks)google/oauth/build.gradle.kts(1 hunks)google/oauth/src/commonMain/kotlin/net/opatry/google/auth/GoogleAuthenticator.kt(1 hunks)google/tasks/build.gradle.kts(1 hunks)gradle/libs.versions.toml(2 hunks)lucide-icons/build.gradle.kts(1 hunks)settings.gradle.kts(1 hunks)tasks-app-ios/Configuration/Config.dev.xcconfig(1 hunks)tasks-app-ios/Configuration/Config.xcconfig(1 hunks)tasks-app-ios/Configuration/Versions.xcconfig(1 hunks)tasks-app-ios/Taskfolio.xcodeproj/project.pbxproj(1 hunks)tasks-app-ios/Taskfolio/Assets.xcassets/AccentColor.colorset/Contents.json(1 hunks)tasks-app-ios/Taskfolio/Assets.xcassets/AccentColor.dev.colorset/Contents.json(1 hunks)tasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Contents.json(1 hunks)tasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Contents.json(1 hunks)tasks-app-ios/Taskfolio/Assets.xcassets/Contents.json(1 hunks)tasks-app-ios/Taskfolio/Configuration/Versions.xcconfig(1 hunks)tasks-app-ios/Taskfolio/ContentView.swift(1 hunks)tasks-app-ios/Taskfolio/IOSGoogleAuthenticator.swift(1 hunks)tasks-app-ios/Taskfolio/Info.plist(1 hunks)tasks-app-ios/Taskfolio/Resources/licenses_ios.json(1 hunks)tasks-app-ios/Taskfolio/TaskfolioApp.swift(1 hunks)tasks-app-ios/build.gradle.kts(1 hunks)tasks-app-shared/build.gradle.kts(2 hunks)tasks-app-shared/src/commonMain/kotlin/net/opatry/tasks/data/TasksAppDatabase.kt(1 hunks)tasks-app-shared/src/iosMain/kotlin/net/opatry/network/NetworkStatusNotifier.ios.kt(1 hunks)tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/FileCredentialsStorage.ios.kt(1 hunks)tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/MainViewController.kt(1 hunks)tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/di/authModule.ios.kt(1 hunks)tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/di/platformModule.ios.kt(1 hunks)tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/initKoin.kt(1 hunks)tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/ui/component/AuthorizeGoogleTasksButton.ios.kt(1 hunks)tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/ui/component/backHandler.ios.kt(1 hunks)tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/ui/theme/Type.ios.kt(1 hunks)tasks-app-shared/src/iosTest/kotlin/net/opatry/tasks/data/util/inMemoryTasksAppDatabaseBuilder.ios.kt(1 hunks)tasks-core/build.gradle.kts(2 hunks)tasks-core/src/iosMain/kotlin/net/opatry/tasks/TaskPosition.ios.kt(1 hunks)website/index.md(2 hunks)website/styles.css(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
tasks-app-ios/Taskfolio/ContentView.swift (1)
tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/MainViewController.kt (1)
MainViewController(44-113)
tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/MainViewController.kt (4)
tasks-app-shared/src/commonMain/kotlin/net/opatry/tasks/app/ui/theme/Theme.kt (1)
TaskfolioTheme(109-124)tasks-app-shared/src/commonMain/kotlin/net/opatry/tasks/app/ui/component/LoadingPane.kt (1)
LoadingPane(36-41)tasks-app-shared/src/commonMain/kotlin/net/opatry/tasks/app/ui/TasksApp.kt (1)
TasksApp(88-175)tasks-app-shared/src/commonMain/kotlin/net/opatry/tasks/app/ui/screen/authorizationScreen.kt (1)
AuthorizationScreen(60-92)
tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/ui/component/AuthorizeGoogleTasksButton.ios.kt (1)
tasks-app-shared/src/commonMain/kotlin/net/opatry/tasks/app/ui/component/LoadingIndicator.kt (1)
LoadingIndicator(38-59)
tasks-app-ios/Taskfolio/IOSGoogleAuthenticator.swift (3)
google/oauth-http/src/commonMain/kotlin/net/opatry/google/auth/HttpGoogleAuthenticator.kt (1)
config(70-249)tasks-app-shared/src/commonMain/kotlin/net/opatry/tasks/app/presentation/UserViewModel.kt (2)
signOut(116-123)signIn(97-114)google/oauth/src/commonMain/kotlin/net/opatry/google/auth/GoogleAuthenticator.kt (1)
refreshToken(85-88)
🪛 detekt (1.23.8)
buildSrc/src/main/kotlin/IosTargetsExt.kt
[warning] 27-29: Usages of lateinit should be avoided.
(detekt.potential-bugs.LateinitUsage)
[warning] 1-62: The file does not contain a package declaration.
(detekt.potential-bugs.MissingPackageDeclaration)
🪛 LanguageTool
website/index.md
[grammar] ~19-~19: There might be a mistake here.
Context: ...- | ---------------------------------- | | 
[grammar] ~20-~20: Use correct spacing
Context: ...) | 
| > ℹ️ The application is also available as ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~22-~22: There might be a mistake here.
Context: ...iOS application as well (using [Compose Multi Platform (aka CMP)](https://www.jetbrains.com/co...
(QB_NEW_EN_OTHER)
[grammar] ~22-~22: There might be a mistake here.
Context: ...w.jetbrains.com/compose-multiplatform/) as UI Toolkit). ## 🎯 Project intentions ...
(QB_NEW_EN)
[grammar] ~22-~22: Use correct spacing
Context: .../compose-multiplatform/) as UI Toolkit). ## 🎯 Project intentions - [x] Showcase my...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~57-~57: There might be a mistake here.
Context: ...tlinlang.org/), Multiplatform (aka KMP) - Android and Desktop are fully supported....
(QB_NEW_EN)
[grammar] ~58-~58: There might be a mistake here.
Context: ...Android and Desktop are fully supported. - iOS wasn't initially planned, but a draf...
(QB_NEW_EN)
[grammar] ~59-~59: There might be a mistake here.
Context: ...ersion is available (use it at your own risk, there might be dragons 🐉). - Web is ...
(QB_NEW_EN_OTHER)
[grammar] ~59-~59: There might be a mistake here.
Context: ...ur own risk, there might be dragons 🐉). - Web is not planned any time soon (contri...
(QB_NEW_EN)
README.md
[grammar] ~12-~12: Use correct spacing
Context: ...s seen as achievable and worth tracking. Refer to the root project's [`build.grad...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~39-~39: Use correct spacing
Context: ...n the 80/20 rule to nail the essentials. | 
[grammar] ~41-~41: There might be a problem here.
Context: ...ng) | | | --------------------------------------...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~42-~42: There might be a mistake here.
Context: ...--------------------------------------- |--------------------------------------- ...
(QB_NEW_EN_OTHER)
[grammar] ~42-~42: Use correct spacing
Context: ...- | ---------------------------------- | 
> [!NOTE] > The application is also availa...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~46-~46: There might be a mistake here.
Context: ...ails?id=net.opatry.tasks.app) > [!NOTE] > The application is also available as a...
(QB_NEW_EN)
[grammar] ~47-~47: There might be a mistake here.
Context: ...iOS application as well (using [Compose Multi Platform (aka CMP)](https://www.jetbrains.com/co...
(QB_NEW_EN_OTHER)
[grammar] ~47-~47: There might be a mistake here.
Context: ...w.jetbrains.com/compose-multiplatform/) as UI Toolkit). ## 🎯 Project intentions ...
(QB_NEW_EN)
[grammar] ~47-~47: Use correct spacing
Context: .../compose-multiplatform/) as UI Toolkit). ## 🎯 Project intentions - [x] Showcase my...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~82-~82: There might be a mistake here.
Context: ...tlinlang.org/), Multiplatform (aka KMP) - Android and Desktop are fully supported....
(QB_NEW_EN)
[grammar] ~83-~83: There might be a mistake here.
Context: ...Android and Desktop are fully supported. - iOS wasn't initially planned, but a draf...
(QB_NEW_EN)
[grammar] ~84-~84: There might be a mistake here.
Context: ...ersion is available (use it at your own risk, there might be dragons 🐉). - Web is ...
(QB_NEW_EN_OTHER)
[grammar] ~84-~84: There might be a mistake here.
Context: ...ur own risk, there might be dragons 🐉). - Web is not planned any time soon (contri...
(QB_NEW_EN)
[grammar] ~187-~187: There might be a mistake here.
Context: ...)
(QB_NEW_EN)
[grammar] ~189-~189: There might be a mistake here.
Context: ...ets the job done. It's provided without guarantees, use at your own risk. Feedback and co...
(QB_NEW_EN_OTHER)
[grammar] ~189-~189: There might be a mistake here.
Context: ...ithout guarantees, use at your own risk. Feedback and contributions are welcome t...
(QB_NEW_EN)
[grammar] ~190-~190: There might be a problem here.
Context: ... risk. Feedback and contributions are welcome though 🤝. > [!NOTE] > iOS support is _opt-in...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~190-~190: Use correct spacing
Context: ... and contributions are welcome though 🤝. > [!NOTE] > iOS support is opt-in and di...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~194-~194: Use articles correctly
Context: ...sn't required. > You can enable it by setting ios.target Gradle property to all, ...
(QB_NEW_EN_OTHER_ERROR_IDS_11)
[grammar] ~194-~194: There might be a mistake here.
Context: ... ios.target Gradle property to all, simulator or device from either `local.properti...
(QB_NEW_EN_OTHER)
[grammar] ~194-~194: There might be a mistake here.
Context: ...er local.properties or CLI using -P. > When building from Xcode, it automatical...
(QB_NEW_EN)
[grammar] ~195-~195: Use correct spacing
Context: ...t=simulatorbased onConfig.xcconfig. <details> <summary>See details…</summary> You can build the :tasks-app-shared` co...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~200-~200: There might be a mistake here.
Context: ...Gradle (to check if everything compiles on Kotlin side): ```bash ./gradlew tasks-...
(QB_NEW_EN)
[grammar] ~200-~200: Use correct spacing
Context: ... if everything compiles on Kotlin side): bash ./gradlew tasks-app-shared:linkDebugFrameworkIosSimulatorArm64 -Pios.target=simulator ### Building & Running from IntelliJ/Android...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~206-~206: Use correct spacing
Context: ...g & Running from IntelliJ/Android Studio You can also use the incubating [Kotlin ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~209-~209: Use commas correctly
Context: ... to run the app on a device or simulator, and enables debugging of Kotlin code ev...
(QB_NEW_EN_OTHER_ERROR_IDS_33)
[grammar] ~209-~209: Use correct spacing
Context: ...in code even when called from iOS/Swift. It builds the Kotlin code as a native fr...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~211-~211: Use correct spacing
Context: ...ift code (see next section for details). ### Building & Running from Xcode For full ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~213-~213: There might be a problem here.
Context: ...ils). ### Building & Running from Xcode For full XCFramework build (to be consumed ...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~215-~215: Use correct spacing
Context: ...debuild(or build directly from Xcode): ```bash cd tasks-app-ios IOS_TARGET=simulator xcodebuild -project Taskfolio.xcodeproj \ -scheme Taskfolio \ -sdk iphonesimulator \ -arch arm64 \ -configuration Debug \ build \ CODE_SIGNING_ALLOWED=NO \ CODE_SIGN_IDENTITY="" \ CODE_SIGNING_REQUIRED=NO ``` This triggers the:tasks-app-shared:emb...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~229-~229: Use correct spacing
Context: ...orkForXcode` Gradle task under the hood. For Xcode integration, it's recommended ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~231-~231: Use correct spacing
Context: ...lugin](https://touchlab.co/xcodekotlin): bash brew install xcode-kotlin xcode-kotlin install When you update Xcode, you'll have to sy...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~238-~238: Use correct spacing
Context: ...e Xcode, you'll have to sync the plugin: bash xcode-kotlin sync If you want to debug the Kotlin code fro...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~246-~246: Use correct spacing
Context: ...in code and see syntax coloring as well.
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🪛 SwiftLint (0.57.0)
tasks-app-ios/Taskfolio/IOSGoogleAuthenticator.swift
[Warning] 32-32: FIXMEs should be resolved (in Jvm impl, the force means &...)
(todo)
[Warning] 61-61: Unused parameter in a closure should be replaced with _
(unused_closure_parameter)
🪛 actionlint (1.7.7)
.github/workflows/ios-app-nightly.yml
42-42: description is required in metadata of "Setup JDK & Gradle" action at "/home/jailuser/git/.github/actions/setup-jdk-gradle/action.yml"
(action)
45-45: property "gradle_module" is not defined in object type {}
(expression)
🪛 YAMLlint (1.37.1)
.github/workflows/ios-app-nightly.yml
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 41-41: wrong indentation: expected 6 but found 4
(indentation)
🪛 markdownlint-cli2 (0.17.2)
README.md
39-39: Inline HTML
Element: abbr
(MD033, no-inline-html)
41-41: Images should have alternate text (alt text)
(MD045, no-alt-text)
41-41: Images should have alternate text (alt text)
(MD045, no-alt-text)
41-41: Images should have alternate text (alt text)
(MD045, no-alt-text)
41-41: Images should have alternate text (alt text)
(MD045, no-alt-text)
130-130: Inline HTML
Element: span
(MD033, no-inline-html)
189-189: Emphasis style
Expected: asterisk; Actual: underscore
(MD049, emphasis-style)
189-189: Emphasis style
Expected: asterisk; Actual: underscore
(MD049, emphasis-style)
193-193: Emphasis style
Expected: asterisk; Actual: underscore
(MD049, emphasis-style)
193-193: Emphasis style
Expected: asterisk; Actual: underscore
(MD049, emphasis-style)
197-197: Inline HTML
Element: details
(MD033, no-inline-html)
198-198: Inline HTML
Element: summary
(MD033, no-inline-html)
228-228: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
⏰ 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: licenses-check / ©️ Check licenses (tasks-app-android, src/main/assets/licenses_android.json)
- GitHub Check: licenses-check / ©️ Check licenses (tasks-app-desktop, src/main/resources/licenses_desktop.json)
- GitHub Check: unit-tests / ✅ Tests
- GitHub Check: build-apps / 🔨 Build - 🍎 iOS App
- GitHub Check: build-apps / 🔨 Build - 📱 Android App
- GitHub Check: build-apps / 🔨 Build - 🖥️ Desktop App
- GitHub Check: coverage / 📊 Coverage
🔇 Additional comments (20)
website/index.md (3)
17-17: Alt text improvement for badge — looks goodClearer, more descriptive alt text.
57-60: KMP targets clarification — good callClear separation of fully supported vs. draft targets reduces confusion.
63-63: Typo fix — good“persistence” correction is correct.
tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/ui/component/backHandler.ios.kt (2)
25-28: Import choice LGTM for iOS (ui.backhandler).Correctly avoids the Android-only activity backhandler.
29-31: Keep the experimental opt-in for Compose MPP BackHandler.
androidx.compose.ui.backhandler.BackHandlerin Compose Multiplatform (v1.8.3) remains an experimental API and still requires@OptIn(ExperimentalComposeUiApi::class)and its import. Only the Android-onlyandroidx.activity.compose.BackHandleris stable and opt-in-free. (github.com, developer.android.com)Likely an incorrect or invalid review comment.
tasks-app-shared/src/iosMain/kotlin/net/opatry/network/NetworkStatusNotifier.ios.kt (1)
39-41: Align the monitor type with “internet” reachability (optional)This avoids reporting “satisfied” for local-only paths.
- val monitor = nw_path_monitor_create() + val monitor = platform.Network.nw_path_monitor_create_with_type( + platform.Network.nw_path_monitor_type_internet + )Likely an incorrect or invalid review comment.
tasks-app-ios/Taskfolio/Assets.xcassets/Contents.json (1)
1-6: LGTM: valid asset catalog manifest.
Looks correct for the root Contents.json.Ensure each Xcode configuration sets ASSETCATALOG_COMPILER_APPICON_NAME appropriately (e.g., AppIcon vs AppIcon.dev) to match the appiconset names.
tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/di/authModule.ios.kt (1)
28-29: No duplicate GoogleAuthenticator bindings on iOS. GoogleAuthenticator is registered only once in initKoin; authModule is intentionally empty and not included.gradle/libs.versions.toml (2)
105-106: Scoped usage of libs.bignum confirmed
Only referenced in the iosMain dependencies block in tasks-core/build.gradle.kts; no other targets include this alias.
28-28: Verify bignum version 0.3.10 is the latest stable
Double-check on Maven Central and the project changelog to confirm there isn’t a newer 0.3.x or 0.4.0 release.tasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Contents.json (2)
112-116: Verify marketing icon file/name and alpha channelEnsure the 1024×1024 “ios-marketing” image exists, has no alpha channel, and consider renaming to a neutral filename (e.g., AppIcon-1024.png) to avoid the legacy “ItunesArtwork@2x.png” confusion.
1-122: All icon assets verified – Running the provided script againsttasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconsetshows no missing PNG files, so there are no Xcode warnings or failures to address.tasks-app-shared/src/iosTest/kotlin/net/opatry/tasks/data/util/inMemoryTasksAppDatabaseBuilder.ios.kt (1)
29-31: LGTM — iOS in-memory Room builder implementedThis unblocks iOS tests relying on an in-memory TasksAppDatabase builder.
google/tasks/build.gradle.kts (1)
31-32: LGTM: central note about iOS targets being added at root is clear.Keeps per-module build files lean and avoids duplication.
google/oauth/build.gradle.kts (1)
31-32: LGTM: helpful comment clarifying iOS target wiringNo functional changes; the note reduces confusion about where iOS targets are added.
tasks-app-ios/Taskfolio/TaskfolioApp.swift (1)
10-14: Handle OAuth redirect in your SwiftUI AppThe proposed call to
IOSGoogleAuthenticator().handleOpenURL(_:)won’t compile—handleOpenURL(_:)isn’t implemented. Instead, forward the callback URL to Google Sign-In directly (or wrap it once you add that method):var body: some Scene { WindowGroup { ContentView() .onOpenURL { url in GIDSignIn.sharedInstance.handle(url) } } }If you’d prefer to keep it on your authenticator, first add:
extension IOSGoogleAuthenticator { func handleOpenURL(_ url: URL) { GIDSignIn.sharedInstance.handle(url) } }then call
.onOpenURL { IOSGoogleAuthenticator().handleOpenURL($0) }.Likely an incorrect or invalid review comment.
tasks-app-shared/build.gradle.kts (1)
163-169: No action required—iosTargets extension is available TheiosTargetsproperty is defined in buildSrc/src/main/kotlin/IosTargetsExt.kt and is visible in this module, so the Gradle configuration will succeed.build.gradle.kts (1)
204-211: VerifyembedAndSignAppleFrameworkForXcodetask exists
No registration ofembedAndSignAppleFrameworkForXcodewas found in:tasks-app-shared—confirm it’s declared in your build scripts under CI or register it explicitly (e.g. viatasks.register(...))..github/workflows/ios-app-nightly.yml (1)
42-43: Local action metadata missing description (actionlint).The step uses ./.github/actions/setup-jdk-gradle, whose action.yml lacks description. Add it there to satisfy actionlint.
Do you want me to open a follow-up patch to update that action’s metadata?
tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/MainViewController.kt (1)
97-102: Good: safe file loading with explicit failure.The Elvis fallback avoids a crashy cast and surfaces a clear error.
| on: | ||
| schedule: | ||
| - cron: '0 2 * * *' | ||
| workflow_dispatch: | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Quote the “on” key to silence YAMLlint truthy warning.
GitHub Actions accepts quoted keys; this keeps linters quiet.
-on:
+"on":
schedule:
- cron: '0 2 * * *'
workflow_dispatch:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| on: | |
| schedule: | |
| - cron: '0 2 * * *' | |
| workflow_dispatch: | |
| "on": | |
| schedule: | |
| - cron: '0 2 * * *' | |
| workflow_dispatch: |
🧰 Tools
🪛 YAMLlint (1.37.1)
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
🤖 Prompt for AI Agents
.github/workflows/ios-app-nightly.yml lines 3-7: the YAML linter reports a
truthy key warning for unquoted top-level key "on"; to fix it, wrap the key in
quotes (i.e., change on: to "on":) so the workflow file uses a quoted key
acceptable to GitHub Actions and silences YAMLlint.
| // can't use by lazy, we need Project.findProperty not accessible there | ||
| @Suppress("ObjectPropertyName") | ||
| private lateinit var _iosTargets: List<String> | ||
|
|
||
| private val localProperties = Properties() |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid global lateinit cache; compute per-project on access
A top-level lateinit shared across builds can bleed between projects and parallel configs. Resolve per access; cost is trivial.
-// can't use by lazy, we need Project.findProperty not accessible there
-@Suppress("ObjectPropertyName")
-private lateinit var _iosTargets: List<String>
+// Resolve per access to avoid cross-project bleed and lateinit pitfalls.
val Project.iosTargets: List<String>
- get() {
- if (!::_iosTargets.isInitialized) {
- _iosTargets = when (getIosTargetedConfiguration()) {
- // We ignore "iosX64", not considered as a use case
- "all" -> listOf("iosArm64", "iosSimulatorArm64")
- "simulator" -> listOf("iosSimulatorArm64")
- "device" -> listOf("iosArm64")
- "none" -> emptyList()
- else -> emptyList()
- }
- }
- return _iosTargets
- }
+ get() = when (getIosTargetedConfiguration()?.trim()?.lowercase(Locale.ROOT)) {
+ // We ignore "iosX64", not considered as a use case
+ "all" -> listOf("iosArm64", "iosSimulatorArm64")
+ "simulator" -> listOf("iosSimulatorArm64")
+ "device" -> listOf("iosArm64")
+ "none" -> emptyList()
+ else -> emptyList()
+ }Also applies to: 48-61
🧰 Tools
🪛 detekt (1.23.8)
[warning] 27-29: Usages of lateinit should be avoided.
(detekt.potential-bugs.LateinitUsage)
🤖 Prompt for AI Agents
In buildSrc/src/main/kotlin/IosTargetsExt.kt around lines 27-31 (and similarly
lines 48-61), remove the top-level lateinit cache and the shared top-level
Properties instance and instead compute values per-Project on access: replace
the global _iosTargets and localProperties with a function or an extension
property that accepts a Project (or uses receiver Project) and constructs a
local Properties and computes the List<String> each time (calling
project.findProperty as needed). Update all usages to call that per-project
accessor so state does not bleed between projects or parallel builds.
| > [!NOTE] | ||
| > The application is also available as a desktop (Jvm) application and an iOS application as well (using [Compose Multi Platform (aka CMP)](https://www.jetbrains.com/compose-multiplatform/) as UI Toolkit). | ||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use the official naming “Compose Multiplatform”.
Minor terminology nit for consistency.
-> The application is also available as a desktop (Jvm) application and an iOS application as well (using [Compose Multi Platform (aka CMP)](https://www.jetbrains.com/compose-multiplatform/) as UI Toolkit).
+> The application is also available as a desktop (JVM) application and an iOS application (using [Compose Multiplatform](https://www.jetbrains.com/compose-multiplatform/) as the UI toolkit).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| > [!NOTE] | |
| > The application is also available as a desktop (Jvm) application and an iOS application as well (using [Compose Multi Platform (aka CMP)](https://www.jetbrains.com/compose-multiplatform/) as UI Toolkit). | |
| > [!NOTE] | |
| > The application is also available as a desktop (JVM) application and an iOS application (using [Compose Multiplatform](https://www.jetbrains.com/compose-multiplatform/) as the UI toolkit). |
🧰 Tools
🪛 LanguageTool
[grammar] ~46-~46: There might be a mistake here.
Context: ...ails?id=net.opatry.tasks.app) > [!NOTE] > The application is also available as a...
(QB_NEW_EN)
[grammar] ~47-~47: There might be a mistake here.
Context: ...iOS application as well (using [Compose Multi Platform (aka CMP)](https://www.jetbrains.com/co...
(QB_NEW_EN_OTHER)
[grammar] ~47-~47: There might be a mistake here.
Context: ...w.jetbrains.com/compose-multiplatform/) as UI Toolkit). ## 🎯 Project intentions ...
(QB_NEW_EN)
[grammar] ~47-~47: Use correct spacing
Context: .../compose-multiplatform/) as UI Toolkit). ## 🎯 Project intentions - [x] Showcase my...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🤖 Prompt for AI Agents
In README.md around lines 46 to 48, the text uses the non-standard name "Compose
Multi Platform (aka CMP)"; update it to the official name "Compose
Multiplatform" (e.g., "Compose Multiplatform" with the same URL) and remove the
"aka CMP" parenthetical so the README uses the correct, consistent terminology.
| actual class FileCredentialsStorage actual constructor(private val filepath: String) : CredentialsStorage { | ||
| @OptIn(BetaInteropApi::class) | ||
| actual override suspend fun load(): TokenCache? { | ||
| return withContext(Dispatchers.IO) { | ||
| val fileManager = NSFileManager.defaultManager | ||
| if (!fileManager.fileExistsAtPath(filepath)) return@withContext null | ||
|
|
||
| val data = NSData.dataWithContentsOfFile(filepath) | ||
| ?: return@withContext null | ||
|
|
||
| val content = NSString.create(data, NSUTF8StringEncoding)?.toString() | ||
| ?: return@withContext null | ||
|
|
||
| runCatching { | ||
| Json.decodeFromString<TokenCache>(content) | ||
| }.getOrNull() | ||
| } | ||
| } | ||
|
|
||
| @OptIn(BetaInteropApi::class) | ||
| actual override suspend fun store(tokenCache: TokenCache) { | ||
| val json = Json { prettyPrint = true } | ||
|
|
||
| val success = withContext(Dispatchers.IO) { | ||
| val nsString = NSString.create(string = json.encodeToString(tokenCache)) | ||
| val data = nsString.dataUsingEncoding(NSUTF8StringEncoding) | ||
| ?: error("Failed to encode JSON to NSData") | ||
|
|
||
| val url = NSURL.fileURLWithPath(filepath) | ||
| data.writeToURL(url, atomically = true) | ||
| } | ||
|
|
||
| if (!success) { | ||
| error("Failed to write token cache to file at $filepath") | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Store secrets in Keychain, not a plain file.
Tokens are sensitive; a file (even with NSFileProtection) risks leakage via backups or device compromise. Prefer Keychain for iOS, or at least set NSFileProtectionComplete on the file.
Do you want a follow-up patch providing a Keychain-backed CredentialsStorage with a file fallback?
🤖 Prompt for AI Agents
In
tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/FileCredentialsStorage.ios.kt
around lines 40-75, the code persists TokenCache to a plain file which is
insecure; replace this with a Keychain-backed implementation (or at minimum add
NSFileProtectionComplete to the file) so tokens are stored in the iOS Keychain.
Implement a KeychainCredentialsStorage that encodes TokenCache to JSON and uses
SecItemAdd / SecItemUpdate / SecItemCopyMatching via Kotlin/Native interop to
store/retrieve the data (use a fixed service/ account keys), return null on
missing item, and throw/log on unrecoverable errors; update the actual class to
use Keychain access and only fall back to secure-file storage if Keychain
operations fail, ensuring proper error handling and tests, and update the
expect/actual wiring so other platforms remain unaffected.
| @OptIn(BetaInteropApi::class) | ||
| actual override suspend fun load(): TokenCache? { | ||
| return withContext(Dispatchers.IO) { | ||
| val fileManager = NSFileManager.defaultManager | ||
| if (!fileManager.fileExistsAtPath(filepath)) return@withContext null | ||
|
|
||
| val data = NSData.dataWithContentsOfFile(filepath) | ||
| ?: return@withContext null | ||
|
|
||
| val content = NSString.create(data, NSUTF8StringEncoding)?.toString() | ||
| ?: return@withContext null | ||
|
|
||
| runCatching { | ||
| Json.decodeFromString<TokenCache>(content) | ||
| }.getOrNull() | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider tolerant decoding and minimal logging on corrupt file.
Using the shared Json with ignoreUnknownKeys avoids spurious null returns on benign schema drift; optionally log once if decoding fails to aid support.
- runCatching {
- Json.decodeFromString<TokenCache>(content)
- }.getOrNull()
+ runCatching {
+ json.decodeFromString<TokenCache>(content)
+ }.onFailure { println("WARN: Failed to decode token cache at $filepath: ${it.message}") }
+ .getOrNull()Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/FileCredentialsStorage.ios.kt
around lines 41 to 57, the current decode uses Json.decodeFromString directly
which returns null on decode errors and loses benign schema-drift data; create
or use a Json instance configured with ignoreUnknownKeys = true for tolerant
decoding, and wrap the decode in runCatching that on failure logs a minimal
one-line warning (e.g., NSLog or the existing logger) with non-sensitive context
about a corrupt token file before returning null so decoding is tolerant but
still minimally observable.
| val url = NSURL.fileURLWithPath(filepath) | ||
| data.writeToURL(url, atomically = true) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Pre-create the parent directory before writing.
writeToURL will fail if the directory doesn’t exist (common on first run).
val url = NSURL.fileURLWithPath(filepath)
+ // Ensure parent directory exists
+ NSFileManager.defaultManager.createDirectoryAtURL(
+ url.URLByDeletingLastPathComponent!!,
+ withIntermediateDirectories = true,
+ attributes = null,
+ error = null
+ )
data.writeToURL(url, atomically = true)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| val url = NSURL.fileURLWithPath(filepath) | |
| data.writeToURL(url, atomically = true) | |
| } | |
| val url = NSURL.fileURLWithPath(filepath) | |
| // Ensure parent directory exists | |
| NSFileManager.defaultManager.createDirectoryAtURL( | |
| url.URLByDeletingLastPathComponent!!, | |
| withIntermediateDirectories = true, | |
| attributes = null, | |
| error = null | |
| ) | |
| data.writeToURL(url, atomically = true) |
🤖 Prompt for AI Agents
In
tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/FileCredentialsStorage.ios.kt
around lines 68 to 70, the code writes data to a file URL without ensuring the
parent directory exists, which causes writeToURL to fail on first run; before
calling data.writeToURL(...), obtain the parent directory URL (or path) and
create it using the file manager with intermediate directories enabled, handle
any creation errors (log/propagate) and only then perform the write so the
directory is present.
| [](https://play.google.com/store/apps/details?id=net.opatry.tasks.app) | ||
|
|
||
| | --------------------------------------- |--------------------------------------- |--------------------------------------- | ---------------------------------- | | ||
| | --------------------------------------- |--------------------------------------- | ---------------------------------- | ---------------------------------- | | ||
| |  |  |  |  | | ||
|
|
||
| > ℹ️ The application is also available as a desktop (Jvm) application and an iOS application as well (using [Compose Multi Platform (aka CMP)](https://www.jetbrains.com/compose-multiplatform/) as UI Toolkit). | ||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Quick lint/preview checks
Please verify no remaining markdown issues and that the screenshots table renders as intended.
Also applies to: 57-60
🏁 Script executed:
#!/bin/bash
# Check for images missing alt text
rg -nP '!\[\]\(' website/index.md || echo "No empty alt-text images found"
# Show the table block to visually inspect header/separator/row spacing
rg -nC2 'GetItOnGooglePlay_Badge_Web_color_English\.png|screens/task_lists_light\.png' website/index.mdLength of output: 1047
Add alt text and table header for screenshots
- Replace each empty alt-text (
) on line 20 with meaningful descriptions, e.g.. - Insert a header row above the separator (line 19), for example:
| Task lists | Groceries | Add task | Home (dark) | | ---------- | --------- | -------- | ----------- |
🧰 Tools
🪛 LanguageTool
[grammar] ~19-~19: There might be a mistake here.
Context: ...- | ---------------------------------- | | 
[grammar] ~20-~20: Use correct spacing
Context: ...) | | > ℹ️ The application is also available as ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~22-~22: There might be a mistake here.
Context: ...iOS application as well (using [Compose Multi Platform (aka CMP)](https://www.jetbrains.com/co...
(QB_NEW_EN_OTHER)
[grammar] ~22-~22: There might be a mistake here.
Context: ...w.jetbrains.com/compose-multiplatform/) as UI Toolkit). ## 🎯 Project intentions ...
(QB_NEW_EN)
[grammar] ~22-~22: Use correct spacing
Context: .../compose-multiplatform/) as UI Toolkit). ## 🎯 Project intentions - [x] Showcase my...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🤖 Prompt for AI Agents
In website/index.md around lines 17 to 23, the screenshot table uses empty alt
text and lacks a header row; replace each image token on line 20 with meaningful
alt text (e.g., "Task lists", "Groceries", "Add task", "Home (dark)") and insert
a header row above the separator on line 19 with corresponding column titles and
a second header separator row (for example: a line with the four column titles,
then a line with dashed separators) so the table has accessible alt text and
proper headers.
| | --------------------------------------- |--------------------------------------- | ---------------------------------- | ---------------------------------- | | ||
| |  |  |  |  | |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Table may render inconsistently without a header row
Some markdown engines require a header line before the separator row. Add an empty header (non-breaking spaces) for consistent rendering.
+| | | | |
| --------------------------------------- |--------------------------------------- | ---------------------------------- | ---------------------------------- |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | --------------------------------------- |--------------------------------------- | ---------------------------------- | ---------------------------------- | | |
| |  |  |  |  | | |
| | | | | | | |
| | --------------------------------------- |--------------------------------------- | ---------------------------------- | ---------------------------------- | | |
| |  |  |  |  | |
🧰 Tools
🪛 LanguageTool
[grammar] ~19-~19: There might be a mistake here.
Context: ...- | ---------------------------------- | | 
[grammar] ~20-~20: Use correct spacing
Context: ...) | | > ℹ️ The application is also available as ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🤖 Prompt for AI Agents
In website/index.md around lines 19 to 20, the markdown table starts with a
separator row which may render inconsistently; add a header row above the
separator (use non-breaking spaces or a simple empty header like ) so the
table has a header line followed by the separator and then the image row,
ensuring consistent rendering across markdown engines.
68d54fb to
6be7c6c
Compare
Job Summary for GradleBuild & Test :: build
|
Job Summary for GradleBuild & Test :: build
|
…se Swift wrapping
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
google/oauth/src/commonMain/kotlin/net/opatry/google/auth/GoogleAuthenticator.kt (3)
43-45: KDoc-specified type doesn’t match implementationDocs say scope is “expressed as a list of Scope” but the property is a String. Either adjust docs or expose a parsed view.
Apply:
- * @property scope The scopes of access granted by the [accessToken] expressed as a list of [Scope]. + * @property scope Space-delimited scopes granted, as returned by Google (e.g., "openid profile email").Optionally add a helper:
val OAuthToken.scopes: Set<GoogleAuthenticator.Scope> get() = scope.split(' ').filter(String::isNotBlank).map { GoogleAuthenticator.Scope(it) }.toSet()
103-108: KDoc param name is wrong and incompleteParam is
grant, notcode; also supports refresh-token flow.Apply:
- * @param code The code obtained through [authorize]. + * @param grant Authorization grant: either an auth code from [authorize] or a refresh token.
91-101: RefinerequestUserAuthorizationcallback to use a concrete payload type
- Change the signature in
GoogleAuthenticator.ktfrom
requestUserAuthorization: (data: Any) -> Unit
to something like
requestUserAuthorization: (payload: AuthorizationPayload) -> Unit
backed by a small sealedAuthorizationPayload(expect/actual) for type safety.- Update the KDoc to clearly describe the payload, e.g.:
@param requestUserAuthorization Callback invoked with platform-specific payload to launch the user authorization UI.- Verify all platform implementations map
force = truetoprompt=consent+access_type=offline(HTTP already does; add this mapping in your iOS integration).tasks-app-shared/src/commonMain/kotlin/net/opatry/tasks/data/TasksAppDatabase.kt (1)
35-44: Wrong Instant import — will not compile. Use kotlinx.datetime.Instant.kotlin.time.Instant has no parse(String); your converters rely on kotlinx.datetime.Instant.
-import kotlin.time.Instant +import kotlinx.datetime.Instant
♻️ Duplicate comments (65)
website/styles.css (1)
157-161: Deduplicate accent color and tighten blockquote spacingReuse a CSS variable for the site accent and avoid extra bottom gap in multi-paragraph quotes. Also consider logical properties for RTL.
Apply:
+/* site-wide accent */ +:root { --accent-color: #6BEACA; } blockquote { - margin-left: 0; - padding-left: 1.5rem; - border-left: 4px solid #6BEACA; + margin-left: 0; + padding-left: 1.5rem; + border-left: 4px solid var(--accent-color); } + +/* avoid double spacing when last child is a <p> */ +blockquote p:last-child { margin-bottom: 0; }Optional RTL-safe variant:
- margin-left: 0; - padding-left: 1.5rem; - border-left: 4px solid var(--accent-color); + margin-inline-start: 0; + padding-inline-start: 1.5rem; + border-inline-start: 4px solid var(--accent-color);Tip (outside this hunk): also switch the link color to use the variable for consistency:
a { color: var(--accent-color); }.README.md (4)
91-91: Use the official “Compose Multiplatform”.Consistency with earlier occurrences.
-- [Jetpack Compose](https://developer.android.com/jetpack/compose), [Multiplatform (aka CMP)](https://www.jetbrains.com/compose-multiplatform/) +- [Jetpack Compose](https://developer.android.com/jetpack/compose), [Compose Multiplatform](https://www.jetbrains.com/compose-multiplatform/)
46-48: Use the official “Compose Multiplatform” naming and tighten the sentence.Matches prior feedback; also fix JVM capitalization.
-> [!NOTE] -> The application is also available as a desktop (Jvm) application and an iOS application as well (using [Compose Multi Platform (aka CMP)](https://www.jetbrains.com/compose-multiplatform/) as UI Toolkit). +> [!NOTE] +> The application is also available as a desktop (JVM) app and an iOS app (using [Compose Multiplatform](https://www.jetbrains.com/compose-multiplatform/) as the UI toolkit).
189-195: Unify emphasis style and improve comma usage.Matches earlier comments about using asterisks for emphasis and adding the missing comma.
-The support of iOS works more or less _as-is_ and gets the job done. It's provided without guarantees, use at your own risk. -Feedback and contributions are welcome though 🤝. +The support of iOS works more or less *as-is* and gets the job done. It's provided without guarantees; use at your own risk. +Feedback and contributions are welcome, though 🤝. -> [!NOTE] -> iOS support is _opt-in_ and disabled by default to avoid unnecessary time and disk usage during the initial Gradle sync when the iOS target isn't required. +> [!NOTE] +> iOS support is *opt-in* and disabled by default to avoid unnecessary time and disk usage during the initial Gradle sync when the iOS target isn't required. > You can enable it by setting `ios.target` Gradle property to `all`, `simulator` or `device` from either `local.properties` or CLI using `-P`. > When building from Xcode, it automatically sets `-Pios.target=simulator` based on `Config.xcconfig`.
200-204: Grammar: add “the” before “Kotlin side”.Already noted in prior reviews.
-You can build the `:tasks-app-shared` code for iOS using Gradle (to check if everything compiles on Kotlin side): +You can build the `:tasks-app-shared` code for iOS using Gradle (to check if everything compiles on the Kotlin side):tasks-app-shared/src/iosMain/kotlin/net/opatry/network/NetworkStatusNotifier.ios.kt (3)
42-45: Guard the update handlerWrap to prevent rare crashes if interop throws.
nw_path_monitor_set_update_handler(monitor) { path -> - val hasInternet = nw_path_get_status(path) == nw_path_status_satisfied - trySend(hasInternet).isSuccess + try { + val hasInternet = nw_path_get_status(path) == nw_path_status_satisfied + trySend(hasInternet).isSuccess + } catch (_: Throwable) { + trySend(false).isSuccess + } }
38-53: Prefer a single shared monitor (hot StateFlow) to avoid N monitorsIf multiple call sites invoke this, you’ll spin up multiple NWPathMonitors. Expose a shared hot StateFlow via DI and reuse it.
Run to find call sites and assess dedup potential:
#!/bin/bash # List call sites (excluding build artifacts) rg -nP '\bnetworkStateFlow\s*\(' -g '!**/build/**' -C2
50-52: Free the NWPathMonitor native reference after cancelCancel stops monitoring but doesn’t drop the C-level ref. Release it to avoid a native leak.
awaitClose { - nw_path_monitor_cancel(monitor) + nw_path_monitor_cancel(monitor) + // Drop native reference to avoid leaks + platform.Network.nw_release(monitor) }lucide-icons/build.gradle.kts (1)
32-33: Centralize the repeated iOS-targets note to avoid drift.
Same comment appears across modules; prefer a single source (e.g., build-logic/convention plugin doc or root README) and link to it.settings.gradle.kts (1)
67-69: Gate iOS module include to macOS to avoid non-mac CI/dev sync issues.
Unconditional include can slow/ break Windows/Linux environments. Gate by OS.-include(":tasks-app-ios") +if (System.getProperty("os.name").contains("Mac", ignoreCase = true)) { + include(":tasks-app-ios") +}tasks-app-ios/Configuration/Versions.xcconfig (1)
1-2: Use Xcode-standard version keys for better tooling compatibility.
Switch to MARKETING_VERSION and CURRENT_PROJECT_VERSION.-BUNDLE_VERSION=1 -BUNDLE_SHORT_VERSION_STRING=1.4.3 +CURRENT_PROJECT_VERSION=1 +MARKETING_VERSION=1.4.3tasks-app-ios/Taskfolio/Configuration/Versions.xcconfig (1)
1-2: Switch to Xcode-standard keys (MARKETING_VERSION, CURRENT_PROJECT_VERSION)Use native keys to keep agvtool/Fastlane and Xcode versioning sane; also simplifies project mapping. Ensure the Gradle writer task updates these keys.
Apply:
-BUNDLE_VERSION=1 -BUNDLE_SHORT_VERSION_STRING=1.4.3 +CURRENT_PROJECT_VERSION=1 +MARKETING_VERSION=1.4.3Outside this file (Gradle task): update the writer to emit CURRENT_PROJECT_VERSION and MARKETING_VERSION instead of the custom keys.
.github/workflows/e2e-tests.yml (1)
10-10: Justify the pinned Ubuntu runner or revert to ubuntu-latestPinning is fine if required (e.g., emulator/KVM quirks). If not strictly needed, prefer ubuntu-latest; otherwise add a rationale comment to avoid churn.
Option A (unpin):
- runs-on: ubuntu-24.04 + runs-on: ubuntu-latestOption B (keep pin, document):
- runs-on: ubuntu-24.04 + # Pinned to 24.04 to align with emulator/KVM setup used elsewhere; change with caution. + runs-on: ubuntu-24.04.gitignore (1)
36-40: Generalize Xcode ignores to reduce maintenanceBroaden the patterns to cover future iOS modules and Xcode files without per-path updates.
Apply:
-tasks-app-ios/Taskfolio.xcodeproj/xcuserdata/*.xcuserdatad -tasks-app-ios/Taskfolio.xcodeproj/project.xcworkspace/contents.xcworkspacedata -tasks-app-ios/Taskfolio.xcodeproj/project.xcworkspace/xcuserdata/*.xcuserdatad -tasks-app-ios/Taskfolio.xcodeproj/project.xcworkspace/xcshareddata/* +tasks-app-ios/**/*.xcuserdatad +tasks-app-ios/**/*.xcworkspace/contents.xcworkspacedata +tasks-app-ios/**/*.xcworkspace/xcshareddata/** +# Optional extras often committed by mistake: +tasks-app-ios/**/*.xcuserstatetasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.dev.appiconset/Contents.json (1)
16-20: Remove obsolete iPhone 29x29 1x slotiPhone 1x assets are deprecated; keeping this often causes missing-file warnings.
Apply:
- { - "filename" : "Icon-App-29x29@1x.png", - "idiom" : "iphone", - "scale" : "1x", - "size" : "29x29" - },tasks-app-ios/Taskfolio/Assets.xcassets/AccentColor.dev.colorset/Contents.json (1)
34-50: Adjust dark mode variant for accessibilityUsing identical RGB in light/dark can strain eyes; dark mode should be slightly muted.
Apply:
"color" : { "color-space" : "srgb", "components" : { "alpha" : "1.000", - "blue" : "0.871", - "green" : "1.000", - "red" : "0.506" + "blue" : "0.750", + "green" : "0.900", + "red" : "0.400" } },tasks-app-ios/Taskfolio/Assets.xcassets/AccentColor.colorset/Contents.json (1)
34-50: Dark mode color identical to light modeTweak dark values to improve contrast and comfort.
Apply:
"color" : { "color-space" : "srgb", "components" : { "alpha" : "1.000", - "blue" : "0.345", - "green" : "0.420", - "red" : "0.000" + "blue" : "0.445", + "green" : "0.520", + "red" : "0.100" } },tasks-app-ios/Taskfolio/Assets.xcassets/AppIcon.appiconset/Contents.json (1)
16-20: Drop iPhone 29x29 1x entrySame rationale as dev icon set; avoids Xcode warnings.
Apply:
- { - "filename" : "Icon-App-29x29@1x.png", - "idiom" : "iphone", - "scale" : "1x", - "size" : "29x29" - },website/index.md (2)
22-22: Wording nit: drop the extra “as well”.Smoother grammar; same meaning.
-> ℹ️ The application is also available as a desktop (Jvm) application and an iOS application as well (using [Compose Multi Platform (aka CMP)](https://www.jetbrains.com/compose-multiplatform/) as UI Toolkit). +> ℹ️ The application is also available as a desktop (Jvm) application and an iOS application (using [Compose Multi Platform (aka CMP)](https://www.jetbrains.com/compose-multiplatform/) as UI Toolkit).
19-20: Fix screenshots table: add header row and alt text (MD045).Adds accessibility and consistent rendering across Markdown engines.
Apply:
+| Task lists | Groceries | Add task | Home (dark) | | --------------------------------------- |--------------------------------------- | ---------------------------------- | ---------------------------------- | -|  |  |  |  | +|  |  |  |  |.github/workflows/build-apps.yml (2)
38-49: Enable Gradle/Konan cache for all matrix entries (remove iOS-only gate).Speeds up Android/Desktop too; safe on Ubuntu.
- - name: Cache Gradle - if: ${{ matrix.gradle_module == 'tasks-app-ios' }} + - name: Cache Gradle uses: actions/cache@v4 with: path: | .gradle $HOME/.m2/repository $HOME/.konan key: gradle-${{ runner.os }}-${{ hashFiles('gradle/libs.versions.toml', 'gradle/wrapper/gradle-wrapper.properties', '**/*.gradle.kts', '**/*.gradle') }} restore-keys: | gradle-${{ runner.os }}-
70-72: Remove unused IOS_TARGET env.The Gradle task hard-codes the target/arch; the env implies configurability that doesn’t exist.
- elif [ "${gradle_module}" = "tasks-app-ios" ]; then - IOS_TARGET=simulator ./gradlew tasks-app-shared:linkDebugFrameworkIosSimulatorArm64 + elif [ "${gradle_module}" = "tasks-app-ios" ]; then + ./gradlew tasks-app-shared:linkDebugFrameworkIosSimulatorArm64tasks-app-ios/Taskfolio/Resources/licenses_ios.json (1)
1-1598: Avoid checking in large generated license blobs. Generate in CI and bundle at build time.This 150KB+ JSON will churn on every dependency bump and bloats the repo.
Proposed approach:
- Add this path to .gitignore and remove file from VCS.
- In Gradle, generate AboutLibraries output and copy it into the iOS bundle during CI/Xcode build.
Example Gradle snippet (to place in tasks-app-ios/build.gradle.kts or a shared build script):
tasks.register<Copy>("bundleIosLicenses") { dependsOn(":tasks-app-shared:exportLibraryDefinitions") from(layout.buildDirectory.file("../tasks-app-shared/build/generated/aboutlibraries/release/libraries.json")) into(layout.projectDirectory.dir("Taskfolio/Resources")) rename { "licenses_ios.json" } }Trigger this from your iOS build job before xcodebuild.
.github/workflows/ios-app-nightly.yml (2)
3-7: Quote “on” to silence YAML truthy warning.-on: +"on": schedule: - cron: '0 2 * * *' workflow_dispatch:
41-73: Fix YAML indentation under steps (parse error) and remove invalid matrix reference.List items must be indented 2 spaces under steps; also there’s no matrix in this job.
- steps: - - uses: actions/checkout@v4 - - uses: ./.github/actions/setup-jdk-gradle + steps: + - uses: actions/checkout@v4 + - uses: ./.github/actions/setup-jdk-gradle - - name: Cache Gradle - if: ${{ matrix.gradle_module == 'tasks-app-ios' }} - uses: actions/cache@v4 + - name: Cache Gradle + uses: actions/cache@v4 with: path: | .gradle $HOME/.m2/repository $HOME/.konan key: gradle-${{ runner.os }}-${{ hashFiles('gradle/libs.versions.toml', 'gradle/wrapper/gradle-wrapper.properties', '**/*.gradle.kts', '**/*.gradle') }} restore-keys: | gradle-${{ runner.os }}- - - name: 🔓 Decrypt secrets - env: - PLAYSTORE_SECRET_PASSPHRASE: ${{ secrets.PLAYSTORE_SECRET_PASSPHRASE }} - run: ./_ci/decrypt_secrets.sh + - name: 🔓 Decrypt secrets + env: + PLAYSTORE_SECRET_PASSPHRASE: ${{ secrets.PLAYSTORE_SECRET_PASSPHRASE }} + run: ./_ci/decrypt_secrets.sh - - name: 🔨 Build - run: | - cd tasks-app-ios - IOS_TARGET=simulator xcodebuild \ - -project Taskfolio.xcodeproj \ - -scheme Taskfolio \ - -sdk iphonesimulator \ - -arch arm64 \ - -configuration Debug \ - build \ - CODE_SIGNING_ALLOWED=NO \ - CODE_SIGN_IDENTITY="" \ - CODE_SIGNING_REQUIRED=NO + - name: 🔨 Build + run: | + cd tasks-app-ios + xcodebuild \ + -project Taskfolio.xcodeproj \ + -scheme Taskfolio \ + -sdk iphonesimulator \ + -destination "platform=iOS Simulator,name=iPhone 15" \ + -arch arm64 \ + -configuration Debug \ + build \ + CODE_SIGNING_ALLOWED=NO \ + CODE_SIGN_IDENTITY="" \ + CODE_SIGNING_REQUIRED=NOtasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/ui/theme/Type.ios.kt (1)
28-44: Adopt Material 3 sizes and remove TODOs for consistent readability.Current body/label sizes are small; align with M3 defaults.
internal actual val Typography: androidx.compose.material3.Typography = androidx.compose.material3.Typography( - displayLarge = TextStyle(fontSize = 57.sp), // TODO adjust - displayMedium = TextStyle(fontSize = 45.sp), // TODO adjust - displaySmall = TextStyle(fontSize = 36.sp), // TODO adjust - headlineLarge = TextStyle(fontSize = 32.sp), // TODO adjust - headlineMedium = TextStyle(fontSize = 28.sp), // TODO adjust - headlineSmall = TextStyle(fontSize = 24.sp), // TODO adjust - titleLarge = TextStyle(fontSize = 22.sp), // TODO adjust - titleMedium = TextStyle(fontSize = 16.sp), // TODO adjust - titleSmall = TextStyle(fontSize = 14.sp), // TODO adjust - bodyLarge = TextStyle(fontSize = 14.sp), - bodyMedium = TextStyle(fontSize = 12.sp), - bodySmall = TextStyle(fontSize = 10.sp), - labelLarge = TextStyle(fontSize = 12.sp), - labelMedium = TextStyle(fontSize = 10.sp), - labelSmall = TextStyle(fontSize = 9.sp), + displayLarge = TextStyle(fontSize = 57.sp), + displayMedium = TextStyle(fontSize = 45.sp), + displaySmall = TextStyle(fontSize = 36.sp), + headlineLarge = TextStyle(fontSize = 32.sp), + headlineMedium = TextStyle(fontSize = 28.sp), + headlineSmall = TextStyle(fontSize = 24.sp), + titleLarge = TextStyle(fontSize = 22.sp), + titleMedium = TextStyle(fontSize = 16.sp), + titleSmall = TextStyle(fontSize = 14.sp), + bodyLarge = TextStyle(fontSize = 16.sp), + bodyMedium = TextStyle(fontSize = 14.sp), + bodySmall = TextStyle(fontSize = 12.sp), + labelLarge = TextStyle(fontSize = 14.sp), + labelMedium = TextStyle(fontSize = 12.sp), + labelSmall = TextStyle(fontSize = 11.sp), )tasks-app-shared/src/commonMain/kotlin/net/opatry/tasks/data/TasksAppDatabase.kt (1)
63-64: exportSchema=true requires Room KSP arg room.schemaLocation.Re-flagging: set the KSP argument in Gradle for all KMP targets to avoid build failures.
#!/bin/bash # Verify that Room schemaLocation is configured somewhere in the build rg -n --pretty -C2 'room\.schemaLocation' -- \ build.gradle.kts settings.gradle.kts gradle.properties \ || rg -n --pretty -C2 'room\.schemaLocation' -g '!**/build/**'If missing, add in the shared build config:
ksp { arg("room.schemaLocation", "$projectDir/schemas") }tasks-core/build.gradle.kts (1)
61-65: Avoid unresolved iosTargets; guard on iosMain presence.Prevents failures when the extra isn’t exported to subprojects.
- if (iosTargets.isNotEmpty()) { - iosMain.dependencies { - implementation(libs.bignum) - } - } + // Add only if iosMain exists (root may add iOS targets dynamically) + findByName("iosMain")?.dependencies { + implementation(libs.bignum) + }buildSrc/build.gradle.kts (1)
10-12: Centralize JVM toolchain; remove duplicates in subprojects.Configure jvmToolchain(17) once (root or a convention plugin) to avoid drift; drop per-module copies.
#!/bin/bash # Find all jvmToolchain declarations to centralize them rg -n --pretty 'jvmToolchain\s*\(' -g '!**/build/**'tasks-app-ios/Configuration/Config.dev.xcconfig (1)
3-7: Quote display name; annotate OAuth IDs.Prevents xcconfig parsing issues and clarifies intent.
-APP_DISPLAY_NAME=T4skf0l10 d3v +APP_DISPLAY_NAME="T4skf0l10 d3v" APP_ID=net.opatry.tasks.app.dev IOS_TARGET=simulator -GCP_CLIENT_ID=191682949161-ockace96gikfsif7hoa9h80p2r096iu4.apps.googleusercontent.com -GCP_REVERSED_CLIENT_ID=com.googleusercontent.apps.191682949161-ockace96gikfsif7hoa9h80p2r096iu4 +// OAuth client IDs for PKCE mobile client; safe to embed. +GCP_CLIENT_ID=191682949161-ockace96gikfsif7hoa9h80p2r096iu4.apps.googleusercontent.com +GCP_REVERSED_CLIENT_ID=com.googleusercontent.apps.191682949161-ockace96gikfsif7hoa9h80p2r096iu4#!/bin/bash # Ensure Info.plist contains the reversed client ID scheme for Google Sign-In callbacks fd -a 'Info.plist' tasks-app-ios | while read -r f; do echo "Checking $f" rg -n '<key>CFBundleURLSchemes</key>' -n -C2 "$f" rg -n 'com\.googleusercontent\.apps\.191682949161-ockace96gikfsif7hoa9h80p2r096iu4' "$f" || echo "MISSING in $f" donetasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/di/authModule.ios.kt (1)
28-29: Silence unused parameter warning on iOS actual.
gcpClientIdis intentionally unused; add a suppression to avoid noise.-actual fun authModule(gcpClientId: String): Module = module {} +@Suppress("UNUSED_PARAMETER") +actual fun authModule(gcpClientId: String): Module = module {}tasks-app-ios/Configuration/Config.xcconfig (1)
3-7: Quote display name and document OAuth client ID.Keeps config consistent and preempts false-positive “secret” scanners.
-APP_DISPLAY_NAME=Taskfolio +APP_DISPLAY_NAME="Taskfolio" APP_ID=net.opatry.tasks.app IOS_TARGET=all -GCP_CLIENT_ID=191682949161-79vl4dcpf9lppj5cj5k79tpqhv5ab10u.apps.googleusercontent.com +// OAuth client ID for mobile (PKCE) clients — public identifier, safe to embed +GCP_CLIENT_ID=191682949161-79vl4dcpf9lppj5cj5k79tpqhv5ab10u.apps.googleusercontent.com GCP_REVERSED_CLIENT_ID=com.googleusercontent.apps.191682949161-79vl4dcpf9lppj5cj5k79tpqhv5ab10utasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/initKoin.kt (1)
32-33: Make Koin initialization idempotent.Avoids IllegalStateException if init is called multiple times (e.g., multi-scene).
import net.opatry.tasks.app.di.utilModule -import org.koin.core.context.startKoin +import org.koin.core.context.startKoin +import org.koin.core.context.GlobalContext import org.koin.dsl.module @Suppress("unused") fun initKoin(googleAuthenticator: GoogleAuthenticator) { - startKoin { + if (GlobalContext.getOrNull() != null) return + startKoin { modules( utilModule, loggingModule, platformModule("ios"), dataModule, module { single { googleAuthenticator } }, networkModule, tasksAppModule, ) } }To verify call sites aren’t accidentally invoking it multiple times, run:
#!/bin/bash rg -nC2 'initKoin\(' tasks-app-ios tasks-app-sharedAlso applies to: 35-50
tasks-app-ios/Taskfolio/TaskfolioApp.swift (1)
6-8: Fix Kotlin bridge call: wrong function name.Use
initKoin(nodoprefix) to match the Kotlin declaration.- InitKoinKt.doInitKoin(googleAuthenticator: IOSGoogleAuthenticator()) + InitKoinKt.initKoin(googleAuthenticator: IOSGoogleAuthenticator())tasks-app-ios/Taskfolio/ContentView.swift (1)
9-10: Clarify no-op updater.Add a brief comment to avoid confusion about lifecycle updates.
- func updateUIViewController(_ uiViewController: UIViewController, context: Context) {} + func updateUIViewController(_ uiViewController: UIViewController, context: Context) { + // No updates needed – ComposeView is self-contained. + }tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/ui/component/backHandler.ios.kt (1)
29-35: Scope the experimental opt-in; don’t leak it to callersMove the opt-in inside the function and pass
enabledexplicitly to avoid forcing upstream modules to opt-in and to match the API.-@ExperimentalComposeUiApi @Composable actual fun MyBackHandler(canNavigateBack: () -> Boolean, navigateBack: () -> Unit) { - BackHandler(canNavigateBack()) { + @OptIn(ExperimentalComposeUiApi::class) + BackHandler(enabled = canNavigateBack()) { navigateBack() } }tasks-app-ios/Taskfolio/Info.plist (1)
5-6: Gate or remove CADisableMinimumFrameDurationOnPhone for release buildsThis key is typically for profiling and can raise App Store review flags. Keep it for Debug only or drop it from Release.
tasks-app-shared/build.gradle.kts (1)
166-168: Locale-stable capitalization for KSP configurationsAvoid default-locale differences when deriving configuration names.
- iosTargets.forEach { iosTarget -> - add("ksp${iosTarget.replaceFirstChar(Char::uppercase)}", libs.androidx.room.compiler) - } + iosTargets.forEach { iosTarget -> + add( + "ksp${iosTarget.replaceFirstChar { it.titlecase(java.util.Locale.ROOT) }}", + libs.androidx.room.compiler, + ) + }build.gradle.kts (1)
146-148: Locale-stable framework baseName generationUse an explicit, locale-agnostic transform.
- val frameworkBaseName = project.name.split('-').joinToString("") { part -> - part.replaceFirstChar(Char::uppercase) - } + val frameworkBaseName = project.name + .split('-') + .joinToString("") { part -> + part.replaceFirstChar { it.titlecase(java.util.Locale.ROOT) } + }tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/di/platformModule.ios.kt (3)
45-66: Prefer Application Support over Documents (and consider backup exclusion).Use NSApplicationSupportDirectory instead of NSDocumentDirectory to store app-internal data; Documents is user-visible and syncs. Optionally set NSURLIsExcludedFromBackupKey on the directory.
-import platform.Foundation.NSDocumentDirectory +import platform.Foundation.NSApplicationSupportDirectory @@ - val documentDirectoryPath = fileManager.URLForDirectory( - directory = NSDocumentDirectory, + val baseDirPath = fileManager.URLForDirectory( + directory = NSApplicationSupportDirectory, inDomain = NSUserDomainMask, appropriateForURL = null, create = true, error = null, - )?.path ?: throw IllegalStateException("Could not find document directory") + )?.path ?: throw IllegalStateException("Could not find application support directory") - ("$documentDirectoryPath/.taskfolio").also { appRootDirPath -> + ("$baseDirPath/.taskfolio").also { appRootDirPath -> if (!fileManager.fileExistsAtPath(appRootDirPath)) { val success = fileManager.createDirectoryAtPath( path = appRootDirPath, withIntermediateDirectories = true, attributes = null, error = null ) check(success) { "Failed to create directory at $appRootDirPath" } } }
68-71: Room DB builder is provided instead of a built database.DI currently returns a Builder; first use will fail. Call build().
single { val dbFilePath = get<String>(named("app_root_dir")) + "/tasks.db" - Room.databaseBuilder<TasksAppDatabase>(dbFilePath) + Room.databaseBuilder<TasksAppDatabase>(dbFilePath).build() }
73-77: Do not store tokens in a plain JSON file; use Keychain.Replace FileCredentialsStorage with a Keychain-backed implementation; at minimum apply NSFileProtectionComplete and keep the file in Application Support. I can provide a KeychainCredentialsStorage actual if desired.
buildSrc/src/main/kotlin/IosTargetsExt.kt (3)
23-23: Missing package declaration.Add a package to avoid namespace issues and satisfy detekt.
-import org.gradle.api.Project +package net.opatry.gradle + +import org.gradle.api.Project
36-45: Compile error: Properties.isEmpty used as a property.Use isEmpty() function.
- if (localProperties.isEmpty) { + if (localProperties.isEmpty()) {
27-31: Avoid global lateinit cache; compute per-Project on access.Shared top-level state can bleed across projects/parallel configs; recompute on each access.
-// can't use by lazy, we need Project.findProperty not accessible there -@Suppress("ObjectPropertyName") -private lateinit var _iosTargets: List<String> - -private val localProperties = Properties() -private fun Project.getIosTargetedConfiguration(): String? { - return findProperty("ios.target") as? String - ?: System.getenv("IOS_TARGET") - ?: run { - if (localProperties.isEmpty) { - val localPropertiesFile = File(rootDir, "local.properties") - if (localPropertiesFile.isFile) { - localPropertiesFile.inputStream().use { reader -> - localProperties.load(reader) - } - } - } - localProperties.getProperty("ios.target") - } -} +private fun Project.getIosTargetedConfiguration(): String? { + return findProperty("ios.target") as? String + ?: System.getenv("IOS_TARGET") + ?: run { + val localProps = Properties() + val localPropsFile = File(rootDir, "local.properties") + if (localPropsFile.isFile) { + localPropsFile.inputStream().use(localProps::load) + } + localProps.getProperty("ios.target") + } +} @@ -val Project.iosTargets: List<String> - get() { - if (!::_iosTargets.isInitialized) { - _iosTargets = when (getIosTargetedConfiguration()) { - // We ignore "iosX64", not considered as a use case - "all" -> listOf("iosArm64", "iosSimulatorArm64") - "simulator" -> listOf("iosSimulatorArm64") - "device" -> listOf("iosArm64") - "none" -> emptyList() - else -> emptyList() - } - } - return _iosTargets - } +val Project.iosTargets: List<String> + get() = when (getIosTargetedConfiguration()?.trim()?.lowercase(Locale.ROOT)) { + // We ignore "iosX64", not considered as a use case + "all" -> listOf("iosArm64", "iosSimulatorArm64") + "simulator" -> listOf("iosSimulatorArm64") + "device" -> listOf("iosArm64") + "none" -> emptyList() + else -> emptyList() + }Add missing import:
+import java.util.LocaleAlso applies to: 48-61
tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/MainViewController.kt (3)
77-81: Avoid re-triggering refresh on recompositions.Scope refresh to a one-shot effect; guard inside.
- if (userState == null) { - LaunchedEffect(userState) { - userViewModel.refreshUserState() - } - } + LaunchedEffect(Unit) { + if (userState == null) userViewModel.refreshUserState() + }
90-102: Memoize AboutApp to avoid redoing I/O on every recomposition.Use remember to cache AboutApp and license loader.
- val aboutApp = AboutApp( - name = appName, - version = fullVersion - ) { + val aboutApp = remember(appName, fullVersion) { + AboutApp( + name = appName, + version = fullVersion + ) { val path = mainBundle.pathForResource("licenses_ios", "json") ?: error("licenses_ios.json not found in bundle") platform.Foundation.NSString.stringWithContentsOfFile( path, encoding = platform.Foundation.NSUTF8StringEncoding, error = null ) ?: error("Failed to load licenses_ios.json from bundle path: $path") - } + } + }
25-31: Add remember import if memoizing AboutApp.Required for remember(...) below.
import androidx.compose.material3.Surface import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.remembertasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/FileCredentialsStorage.ios.kt (3)
53-55: Tolerant decoding and minimal logging on corrupt file.Use a shared Json with ignoreUnknownKeys and log once on failure.
- runCatching { - Json.decodeFromString<TokenCache>(content) - }.getOrNull() + runCatching { + json.decodeFromString<TokenCache>(content) + }.onFailure { println("WARN: Failed to decode token cache at $filepath: ${it.message}") } + .getOrNull()
61-66: Avoid new Json per call; ensure parent dir; don’t throw inside withContext.Reuse Json, create parent directory, and return a boolean from IO block.
- actual override suspend fun store(tokenCache: TokenCache) { - val json = Json { prettyPrint = true } - - val success = withContext(Dispatchers.IO) { - val nsString = NSString.create(string = json.encodeToString(tokenCache)) - val data = nsString.dataUsingEncoding(NSUTF8StringEncoding) - ?: error("Failed to encode JSON to NSData") - - val url = NSURL.fileURLWithPath(filepath) - data.writeToURL(url, atomically = true) - } - - if (!success) { - error("Failed to write token cache to file at $filepath") - } - } + actual override suspend fun store(tokenCache: TokenCache) { + val ok = withContext(Dispatchers.IO) { + val nsString = NSString.create(string = json.encodeToString(tokenCache)) + val data = nsString.dataUsingEncoding(NSUTF8StringEncoding) ?: return@withContext false + val url = NSURL.fileURLWithPath(filepath) + // Ensure parent directory exists + val parent = url.URLByDeletingLastPathComponent + if (parent != null) { + NSFileManager.defaultManager.createDirectoryAtURL( + parent, withIntermediateDirectories = true, attributes = null, error = null + ) + } + data.writeToURL(url, atomically = true) + } + if (!ok) { + println("ERROR: Failed to write token cache to file at $filepath") + error("Failed to write token cache to file at $filepath") + } + }Add companion object:
actual class FileCredentialsStorage actual constructor(private val filepath: String) : CredentialsStorage { + companion object { + val json = Json { prettyPrint = true; ignoreUnknownKeys = true } + }Also applies to: 68-74
40-75: Security: store tokens in Keychain, not a plain file.A file risks leakage (backups, device compromise). Replace with Keychain-backed storage or at least apply NSFileProtectionComplete. I can provide a KeychainCredentialsStorage actual.
tasks-app-ios/build.gradle.kts (3)
60-66: Ensure parent directory exists and declare inputs for up-to-date checks.Create Taskfolio/Configuration before write and use inputs.property for content hashing.
outputs.upToDateWhen { configFile.takeIf(File::exists)?.readText() == content } doLast { + // Guarantee directory exists on clean checkouts + configFile.parentFile.mkdirs() configFile.writeText(content) }Optionally:
-tasks.register("updateXcodeVersionConfig") { +tasks.register("updateXcodeVersionConfig") { val configFile = file("${projectDir}/Taskfolio/Configuration/Versions.xcconfig") outputs.file(configFile) - val content = """ + val content = """ BUNDLE_VERSION=$appVersionCode BUNDLE_SHORT_VERSION_STRING=$appVersion """.trimIndent() + inputs.property("content", content)
73-79: offlineMode=true contradicts fetchRemoteLicense=true.If offline, disable remote fetches.
- offlineMode = true - fetchRemoteLicense = true + offlineMode = true + fetchRemoteLicense = false
86-88: Typo: ASDKL license identifier casing.Fix to ASDKL to pass strict check.
- allowedLicenses.addAll("Apache-2.0", "asdkl", "MIT", "EPL-1.0", "BSD-3-Clause") + allowedLicenses.addAll("Apache-2.0", "ASDKL", "MIT", "EPL-1.0", "BSD-3-Clause")tasks-app-shared/src/iosMain/kotlin/net/opatry/tasks/app/ui/component/AuthorizeGoogleTasksButton.ios.kt (3)
61-81: Reset stale error, handle cancellation, and always clear loading via finally.Keeps UI responsive and avoids surfacing user cancellations as errors.
Button( onClick = { - ongoingAuth = true + error = null + ongoingAuth = true coroutineScope.launch { val scopes = listOf( GoogleAuthenticator.Scope.Profile, GoogleAuthenticator.Scope(TasksScopes.Tasks), ) - try { + try { val authCode = authenticator.authorize( scopes = scopes, force = true, requestUserAuthorization = {}, ).let(GoogleAuthenticator.Grant::AuthorizationCode) val oauthToken = authenticator.getToken(authCode) onSuccess(oauthToken) - } catch (e: Exception) { - error = e.message - ongoingAuth = false - } + } catch (e: kotlinx.coroutines.CancellationException) { + // cancellation is expected; don't surface to the user + throw e + } catch (e: Exception) { + error = e.message + } finally { + ongoingAuth = false + } } }, enabled = !ongoingAuth ) {
85-93: Don’t reuse parent modifier on inner Box.
Prevents unintended sizing constraints leaking in.- Box(modifier, contentAlignment = Alignment.Center) { + Box(Modifier, contentAlignment = Alignment.Center) {
96-98: Render error only when non-empty.
Avoids empty placeholder occupying space.- AnimatedContent(error, label = "authorize_error_message") { message -> - Text(message ?: "", color = MaterialTheme.colorScheme.error) - } + androidx.compose.animation.AnimatedVisibility(visible = !((error ?: "").isBlank())) { + Text(error!!, color = MaterialTheme.colorScheme.error) + }tasks-app-ios/Taskfolio.xcodeproj/project.pbxproj (4)
258-258: Lower iOS deployment target for reach.
18.2 is restrictive; consider 17.0 unless dependencies mandate 18.x.- IPHONEOS_DEPLOYMENT_TARGET = 18.2; + IPHONEOS_DEPLOYMENT_TARGET = 17.0;Apply in both Debug and Release.
Also applies to: 319-319
342-342: Enable user script sandboxing (security).
Currently disabled at target level, overriding the project-level YES.- ENABLE_USER_SCRIPT_SANDBOXING = NO; + ENABLE_USER_SCRIPT_SANDBOXING = YES;Also apply to Release.
Also applies to: 383-383
169-187: Make Kotlin framework phase incremental and robust.
Avoid always-out-of-date runs and harden the script.473FF7702E05F9E6002B3B0F /* Compile Kotlin Framework */ = { isa = PBXShellScriptBuildPhase; - alwaysOutOfDate = 1; + alwaysOutOfDate = 0; ... - shellScript = "if [ \"${OVERRIDE_KOTLIN_BUILD_IDE_SUPPORTED}\" = \"YES\" ]; then\n echo \"Skipping Gradle build task invocation due to OVERRIDE_KOTLIN_BUILD_IDE_SUPPORTED environment variable set to 'YES'.\"\n exit 0\nfi\n\nif [ -z \"${IOS_TARGET}\" ]; then\n echo \"You must define IOS_TARGET to 'all', 'simulator' or 'device' to allow building for iOS.\"\n exit 1\nfi\n\necho \"Building for '${IOS_TARGET}' target\"\n\ncd \"${SRCROOT}/..\"\n./gradlew :tasks-app-shared:embedAndSignAppleFrameworkForXcode\n"; + shellScript = "set -euo pipefail\n\nif [ \"${OVERRIDE_KOTLIN_BUILD_IDE_SUPPORTED}\" = \"YES\" ]; then\n echo \"Skipping Gradle build (OVERRIDE_KOTLIN_BUILD_IDE_SUPPORTED=YES).\"\n exit 0\nfi\n\ncase \"${IOS_TARGET:-}\" in\n all|simulator|device) ;; \n *) echo \"ERROR: IOS_TARGET must be one of: all|simulator|device\"; exit 1;;\nesac\n\necho \"Building for '${IOS_TARGET}' (CONFIG=${CONFIGURATION} SDK=${SDK_NAME} ARCHS=${ARCHS})\"\ncd \"${SRCROOT}/..\"\n\nif [ ! -x ./gradlew ]; then echo \"ERROR: gradlew not found\"; exit 1; fi\n./gradlew :tasks-app-shared:embedAndSignAppleFrameworkForXcode \\\n -PXCODE_CONFIGURATION=${CONFIGURATION} \\\n -PXCODE_SDK_NAME=${SDK_NAME} \\\n -PXCODE_ARCHS=\"${ARCHS}\"\n";Optional: add output tracking to reduce re-runs
- outputPaths:
$(SRCROOT)/../tasks-app-shared/build/xcode-frameworks/$ (CONFIGURATION)/$(SDK_NAME)/TasksAppShared.framework
340-340: Move Development Team to xcconfig.
Avoid hardcoding team ID in the project.- DEVELOPMENT_TEAM = 9J8LQ9R327; + DEVELOPMENT_TEAM = $(DEVELOPMENT_TEAM);Then define in your xcconfig(s):
/* Configuration/Config.dev.xcconfig (or Config.xcconfig) */ DEVELOPMENT_TEAM = 9J8LQ9R327Also applies to: 381-381
tasks-app-ios/Taskfolio/IOSGoogleAuthenticator.swift (4)
35-37: Revisit “force” semantics.
Signing out every time is heavy; consider revoking only when you truly need to re-consent/offline access.Do you intend force to always prompt consent, only on first sign-in, or when scopes change? I can align behavior across Android/JVM/iOS accordingly.
61-61: Replace unused closure parameter with underscore.
Minor clarity and lint fix.- user.addScopes(stringScopes, presenting: rootViewController) { scopeResult, scopeError in + user.addScopes(stringScopes, presenting: rootViewController) { _, scopeError in
119-132: Remove unused error case.
configurationFailed is never thrown.enum GoogleSignInError: Error, LocalizedError { case noRootViewController case missingConfiguration - case configurationFailed case userNotSignedIn case tokenNotAvailable case unknownError @@ - case .configurationFailed: - return "Failed to configure Google Sign-In"
39-53: Extract duplicated “add scopes” logic.
Reduces repetition and centralizes error handling.I can provide a small helper:
private func addScopesIfNeeded(_ scopes: [String], for user: GIDGoogleUser, presenting vc: UIViewController, resume: @escaping (Result<Void, Error>) -> Void) { guard !scopes.isEmpty else { return resume(.success(())) } user.addScopes(scopes, presenting: vc) { _, err in if let err { resume(.failure(err)) } else { resume(.success(())) } } }Then call it in both branches.
Also applies to: 60-67
| gradle.projectsEvaluated { | ||
| val xcFrameworkTask = project(":tasks-app-shared").tasks.findByName("embedAndSignAppleFrameworkForXcode") | ||
| val updateVersionTask = project(":tasks-app-ios").tasks.findByName("updateXcodeVersionConfig") | ||
|
|
||
| if (xcFrameworkTask != null && updateVersionTask != null) { | ||
| xcFrameworkTask.dependsOn(updateVersionTask) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Task wiring OK
Dependency ensures version config is generated before embedding the framework. Consider tasks.named(...).configure { dependsOn(...) } for more eager validation, but current approach is acceptable.
🤖 Prompt for AI Agents
In build.gradle.kts around lines 204 to 211, the current wiring uses
tasks.findByName inside gradle.projectsEvaluated which defers validation; change
to using tasks.named(...) and configure { dependsOn(...) } so Gradle resolves
the task references eagerly and fails fast if tasks are missing, ensuring the
dependency is set up reliably.
| // Note: iOS targets are conditionally added dynamically in the root build.gradle.kts | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Clarify the source of iOS target configuration in-place
Good call to note dynamic iOS setup. Make the comment actionable by pointing to the exact source and knobs devs can use.
Apply:
- // Note: iOS targets are conditionally added dynamically in the root build.gradle.kts
+ // iOS targets are added in the root build.gradle.kts via buildSrc/IosTargetsExt.kt.
+ // Configure with: Gradle property `ios.target` or env `IOS_TARGET` (all|simulator|device|none).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Note: iOS targets are conditionally added dynamically in the root build.gradle.kts | |
| // iOS targets are added in the root build.gradle.kts via buildSrc/IosTargetsExt.kt. | |
| // Configure with: Gradle property `ios.target` or env `IOS_TARGET` (all|simulator|device|none). |
🤖 Prompt for AI Agents
In google/oauth/build.gradle.kts around lines 31–32, the note about iOS targets
being added dynamically should be made actionable: update the comment to point
developers to the exact location in the repository (the root build.gradle.kts)
where the conditional logic lives (search for the function or block that adds
iOS targets, e.g., the "addIosTargets" / "enableIosTargets" logic), and list the
knobs to change (the project property or Gradle flag used to enable/disable iOS
targets and where to set it — gradle.properties, environment variable, or via -P
on the command line), plus a short example of how to toggle it (e.g., set
property in gradle.properties or run ./gradlew assemble -PiosTargets=true).
| [**Taskfolio**](https://opatry.github.io/taskfolio) is an Android task management app built using [Google Tasks API](https://developers.google.com/tasks/reference/rest). Developed to demonstrate my expertise in modern Android development, it highlights my skills in architecture, UI design with Jetpack Compose, OAuth authentication, and more—all packaged in a sleek, user-friendly interface. | ||
|
|
||
| > I set out to revisit the classical TODO app, ‘local-first’ syncing with Google Tasks—aiming for an <abbr title="Minimum Viable Experience">MVE</abbr> in 2 weeks, focusing on the 80/20 rule to nail the essentials. | ||
| > I set out to revisit the classical TODO app, 'local-first' syncing with Google Tasks—aiming for an <abbr title="Minimum Viable Experience">MVE</abbr> in 2 weeks, focusing on the 80/20 rule to nail the essentials. |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Replace inline HTML with plain text for markdownlint compatibility.
Avoid MD033 by expanding the abbreviation inline.
-> I set out to revisit the classical TODO app, 'local-first' syncing with Google Tasks—aiming for an <abbr title="Minimum Viable Experience">MVE</abbr> in 2 weeks, focusing on the 80/20 rule to nail the essentials.
+> I set out to revisit the classical TODO app, 'local-first' syncing with Google Tasks—aiming for a Minimum Viable Experience (MVE) in 2 weeks, focusing on the 80/20 rule to nail the essentials.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| > I set out to revisit the classical TODO app, 'local-first' syncing with Google Tasks—aiming for an <abbr title="Minimum Viable Experience">MVE</abbr> in 2 weeks, focusing on the 80/20 rule to nail the essentials. | |
| > I set out to revisit the classical TODO app, 'local-first' syncing with Google Tasks—aiming for a Minimum Viable Experience (MVE) in 2 weeks, focusing on the 80/20 rule to nail the essentials. |
🧰 Tools
🪛 LanguageTool
[grammar] ~39-~39: Use correct spacing
Context: ...n the 80/20 rule to nail the essentials. | 
🪛 markdownlint-cli2 (0.17.2)
39-39: Inline HTML
Element: abbr
(MD033, no-inline-html)
🤖 Prompt for AI Agents
In README.md around line 39, replace the inline HTML <abbr> usage with plain
Markdown text to avoid MD033; expand the abbreviation inline by changing "an
<abbr title=\"Minimum Viable Experience\">MVE</abbr>" to "an MVE (Minimum Viable
Experience)" so the abbreviation is presented without HTML while preserving the
same meaning.
| ## 🛠️ Tech stack | ||
|
|
||
| - [Kotlin](https://kotlinlang.org/), [Multiplatform (aka KMP)](https://kotlinlang.org/docs/multiplatform.html) (currently Desktop & Android are supported) | ||
| - iOS wasn’t initially planned, but I bootstrapped a [PR to evaluate the feasibility of the iOS target]((https://github.com/opatry/taskfolio/pull/269)). It turned out to be quite achievable and just needs some polishing. | ||
| - Web is not planned any time soon (contribution are welcome 🤝) | ||
| - [Kotlin](https://kotlinlang.org/), [Multiplatform (aka KMP)](https://kotlinlang.org/docs/multiplatform.html) | ||
| - Android and Desktop are fully supported. | ||
| - iOS wasn't initially planned, but a draft version is available (use it at your own risk, there might be dragons 🐉). | ||
| - Web is not planned any time soon (contributions are welcome 🤝) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Minor terminology and grammar cleanups.
Optional: inline KMP, keep plural “contributions”.
-- [Kotlin](https://kotlinlang.org/), [Multiplatform (aka KMP)](https://kotlinlang.org/docs/multiplatform.html)
+- [Kotlin](https://kotlinlang.org/), [Kotlin Multiplatform (KMP)](https://kotlinlang.org/docs/multiplatform.html)
- Android and Desktop are fully supported.
- - iOS wasn't initially planned, but a draft version is available (use it at your own risk, there might be dragons 🐉).
- - Web is not planned any time soon (contributions are welcome 🤝)
+ - iOS wasn't initially planned, but a draft is available (use it at your own risk; there might be dragons 🐉).
+ - Web is not planned any time soon (contributions are welcome 🤝)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## 🛠️ Tech stack | |
| - [Kotlin](https://kotlinlang.org/), [Multiplatform (aka KMP)](https://kotlinlang.org/docs/multiplatform.html) (currently Desktop & Android are supported) | |
| - iOS wasn’t initially planned, but I bootstrapped a [PR to evaluate the feasibility of the iOS target]((https://github.com/opatry/taskfolio/pull/269)). It turned out to be quite achievable and just needs some polishing. | |
| - Web is not planned any time soon (contribution are welcome 🤝) | |
| - [Kotlin](https://kotlinlang.org/), [Multiplatform (aka KMP)](https://kotlinlang.org/docs/multiplatform.html) | |
| - Android and Desktop are fully supported. | |
| - iOS wasn't initially planned, but a draft version is available (use it at your own risk, there might be dragons 🐉). | |
| - Web is not planned any time soon (contributions are welcome 🤝) | |
| ## 🛠️ Tech stack | |
| - [Kotlin](https://kotlinlang.org/), [Kotlin Multiplatform (KMP)](https://kotlinlang.org/docs/multiplatform.html) | |
| - Android and Desktop are fully supported. | |
| - iOS wasn't initially planned, but a draft is available (use it at your own risk; there might be dragons 🐉). | |
| - Web is not planned any time soon (contributions are welcome 🤝) |
🧰 Tools
🪛 LanguageTool
[grammar] ~80-~80: There might be a mistake here.
Context: ...skfolio/issues/155)). ## 🛠️ Tech stack - Kotlin, [Multi...
(QB_NEW_EN)
[grammar] ~82-~82: There might be a mistake here.
Context: ...tlinlang.org/), Multiplatform (aka KMP) - Android and Desktop are fully supported....
(QB_NEW_EN)
[grammar] ~83-~83: There might be a mistake here.
Context: ...Android and Desktop are fully supported. - iOS wasn't initially planned, but a draf...
(QB_NEW_EN)
[grammar] ~84-~84: There might be a mistake here.
Context: ...ersion is available (use it at your own risk, there might be dragons 🐉). - Web is ...
(QB_NEW_EN_OTHER)
[grammar] ~84-~84: There might be a mistake here.
Context: ...ur own risk, there might be dragons 🐉). - Web is not planned any time soon (contri...
(QB_NEW_EN)
🤖 Prompt for AI Agents
In README.md around lines 80 to 85, change the tech stack wording to use "Kotlin
Multiplatform (KMP)" inline instead of separate Multiplatform link, fix minor
grammar and punctuation (e.g., replace the comma before the parenthetical about
iOS with ", but a draft version is available (use at your own risk; there may be
dragons 🐉)." ), and keep "contributions" plural in the Web line; update the
bullet to read clearly and concisely while preserving links and emojis.
| - [`:tasks-app-ios/Taskfolio`](tasks-app-ios/Taskfolio) <span style="color: #33FF00;">■■■■■■■■■</span>□ 90% | ||
| - The iOS application (thin layer fully reusing `:tasks-app-shared`) | ||
| - Xcode project, written in Swift |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Clarify iOS module composition.
Tiny wording tweak; optional.
- - The iOS application (thin layer fully reusing `:tasks-app-shared`)
- - Xcode project, written in Swift
+ - The iOS application (thin layer fully reusing `:tasks-app-shared`)
+ - Xcode project, primarily Swift, integrating the shared Kotlin framework📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - [`:tasks-app-ios/Taskfolio`](tasks-app-ios/Taskfolio) <span style="color: #33FF00;">■■■■■■■■■</span>□ 90% | |
| - The iOS application (thin layer fully reusing `:tasks-app-shared`) | |
| - Xcode project, written in Swift | |
| - [`:tasks-app-ios/Taskfolio`](tasks-app-ios/Taskfolio) <span style="color: #33FF00;">■■■■■■■■■</span>□ 90% | |
| - The iOS application (thin layer fully reusing `:tasks-app-shared`) | |
| - Xcode project, primarily Swift, integrating the shared Kotlin framework |
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
130-130: Inline HTML
Element: span
(MD033, no-inline-html)
🤖 Prompt for AI Agents
In README.md around lines 130 to 132, the wording for the iOS module is
ambiguous; update the line for `:tasks-app-ios/Taskfolio` to clearly state that
this is a thin iOS app layer that reuses the `:tasks-app-shared` module for
business logic and components, and mention it's an Xcode project written in
Swift (e.g., "Thin iOS app layer reusing :tasks-app-shared; Xcode project in
Swift") to clarify composition.
| actual override val value: String | ||
| get() = rawValue.toString().padStart(20, '0') |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Extract magic number 20 into a single width constant.
Avoid duplicating the padding width and make it self-documenting.
package net.opatry.tasks
import com.ionspin.kotlin.bignum.integer.BigInteger
import com.ionspin.kotlin.bignum.integer.toBigInteger
import kotlinx.datetime.Instant
+private const val POSITION_STRING_WIDTH = 20
+
actual class TodoTaskPosition private constructor(internal val rawValue: BigInteger) : TaskPosition {
actual override val value: String
- get() = rawValue.toString().padStart(20, '0')
+ get() = rawValue.toString().padStart(POSITION_STRING_WIDTH, '0')
...
actual class DoneTaskPosition private constructor(internal val rawValue: BigInteger) : TaskPosition {
actual override val value: String
- get() = rawValue.toString().padStart(20, '0')
+ get() = rawValue.toString().padStart(POSITION_STRING_WIDTH, '0')Also applies to: 76-77
🤖 Prompt for AI Agents
In tasks-core/src/iosMain/kotlin/net/opatry/tasks/TaskPosition.ios.kt around
lines 30-31 (and similarly at lines 76-77), the magic number 20 is used directly
for padStart width; extract this into a single descriptive constant (e.g.,
PADDING_WIDTH or ID_PAD_WIDTH) declared at the top of the file or companion
object, replace the literal 20 in both padStart calls with that constant, and
ensure the constant is documented by its name so the padding width is
centralized and self-explanatory.
| actual fun fromIndex(index: Int): TodoTaskPosition { | ||
| return TodoTaskPosition(BigInteger.fromInt(index)) | ||
| } |
There was a problem hiding this comment.
Validate index >= 0 in fromIndex().
Negative indices will parse and serialize fine but are likely invalid for ordering; fail fast.
actual companion object {
actual fun fromIndex(index: Int): TodoTaskPosition {
- return TodoTaskPosition(BigInteger.fromInt(index))
+ require(index >= 0) { "index must be >= 0" }
+ return TodoTaskPosition(BigInteger.fromInt(index))
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| actual fun fromIndex(index: Int): TodoTaskPosition { | |
| return TodoTaskPosition(BigInteger.fromInt(index)) | |
| } | |
| actual companion object { | |
| actual fun fromIndex(index: Int): TodoTaskPosition { | |
| require(index >= 0) { "index must be >= 0" } | |
| return TodoTaskPosition(BigInteger.fromInt(index)) | |
| } | |
| } |
🤖 Prompt for AI Agents
In tasks-core/src/iosMain/kotlin/net/opatry/tasks/TaskPosition.ios.kt around
lines 34 to 36, the fromIndex(index: Int) function currently accepts negative
indices which are invalid for ordering; add a validation that index >= 0 and
fail fast if not. Implement a check (e.g. use require(index >= 0) or explicitly
throw IllegalArgumentException with a clear message like "index must be
non-negative") before constructing the BigInteger so negative values are
rejected immediately.
| actual fun fromPosition(position: String): TodoTaskPosition { | ||
| return TodoTaskPosition(BigInteger.parseString(position)) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add strict input validation for fromPosition() in both classes.
Reject non-digits and oversize inputs; keeps data canonical and prevents runtime parse errors.
- actual fun fromPosition(position: String): TodoTaskPosition {
- return TodoTaskPosition(BigInteger.parseString(position))
- }
+ actual fun fromPosition(position: String): TodoTaskPosition {
+ require(position.isNotEmpty() && position.all(Char::isDigit)) { "Invalid position: must be decimal digits" }
+ require(position.length <= POSITION_STRING_WIDTH) { "Invalid position: max $POSITION_STRING_WIDTH digits" }
+ return TodoTaskPosition(BigInteger.parseString(position))
+ }
...
- actual fun fromPosition(position: String): DoneTaskPosition {
- return DoneTaskPosition(BigInteger.parseString(position))
- }
+ actual fun fromPosition(position: String): DoneTaskPosition {
+ require(position.isNotEmpty() && position.all(Char::isDigit)) { "Invalid position: must be decimal digits" }
+ require(position.length <= POSITION_STRING_WIDTH) { "Invalid position: max $POSITION_STRING_WIDTH digits" }
+ return DoneTaskPosition(BigInteger.parseString(position))
+ }Also applies to: 71-73
| actual override fun compareTo(other: TaskPosition): Int { | ||
| return when (other) { | ||
| is TodoTaskPosition -> rawValue.compareTo(other.rawValue) | ||
| is DoneTaskPosition -> rawValue.compareTo(other.rawValue) | ||
| else -> throw IllegalArgumentException("Only TodoTaskPosition and DoneTaskPosition are supported") | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Deduplicate compareTo() logic and improve error message.
+private fun compareTaskPositions(thisValue: BigInteger, other: TaskPosition): Int =
+ when (other) {
+ is TodoTaskPosition -> thisValue.compareTo(other.rawValue)
+ is DoneTaskPosition -> thisValue.compareTo(other.rawValue)
+ else -> throw IllegalArgumentException("Unsupported TaskPosition type: ${other::class.simpleName}")
+ }
+
actual override fun compareTo(other: TaskPosition): Int {
- return when (other) {
- is TodoTaskPosition -> rawValue.compareTo(other.rawValue)
- is DoneTaskPosition -> rawValue.compareTo(other.rawValue)
- else -> throw IllegalArgumentException("Only TodoTaskPosition and DoneTaskPosition are supported")
- }
+ return compareTaskPositions(rawValue, other)
}
...
actual override fun compareTo(other: TaskPosition): Int {
- return when (other) {
- is TodoTaskPosition -> rawValue.compareTo(other.rawValue)
- is DoneTaskPosition -> rawValue.compareTo(other.rawValue)
- else -> throw IllegalArgumentException("Only TodoTaskPosition and DoneTaskPosition are supported")
- }
+ return compareTaskPositions(rawValue, other)
}Also applies to: 79-85
| private val UpperBound = BigInteger.parseString("9999999999999999999") | ||
| actual fun fromCompletionDate(completionDate: Instant): DoneTaskPosition { | ||
| return DoneTaskPosition(UpperBound - completionDate.toEpochMilliseconds().toBigInteger()) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Derive and rename upper bound from width to avoid drift.
Pad width is 20 but UpperBound encodes 19 nines. Compute from the single width constant and use constant-style naming.
- private val UpperBound = BigInteger.parseString("9999999999999999999")
+ // Maximum 20-digit value (all 9s), derived from POSITION_STRING_WIDTH
+ private val UPPER_BOUND = BigInteger.parseString("9".repeat(POSITION_STRING_WIDTH))
actual fun fromCompletionDate(completionDate: Instant): DoneTaskPosition {
- return DoneTaskPosition(UpperBound - completionDate.toEpochMilliseconds().toBigInteger())
+ return DoneTaskPosition(UPPER_BOUND - completionDate.toEpochMilliseconds().toBigInteger())
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private val UpperBound = BigInteger.parseString("9999999999999999999") | |
| actual fun fromCompletionDate(completionDate: Instant): DoneTaskPosition { | |
| return DoneTaskPosition(UpperBound - completionDate.toEpochMilliseconds().toBigInteger()) | |
| } | |
| // Maximum 20-digit value (all 9s), derived from POSITION_STRING_WIDTH | |
| private val UPPER_BOUND = BigInteger.parseString("9".repeat(POSITION_STRING_WIDTH)) | |
| actual fun fromCompletionDate(completionDate: Instant): DoneTaskPosition { | |
| return DoneTaskPosition(UPPER_BOUND - completionDate.toEpochMilliseconds().toBigInteger()) | |
| } |
🤖 Prompt for AI Agents
In tasks-core/src/iosMain/kotlin/net/opatry/tasks/TaskPosition.ios.kt around
lines 66 to 69, the UpperBound constant is hardcoded as 19 nines while pad width
is 20; replace the hardcoded value with a computed constant derived from a
single width constant (e.g., PAD_WIDTH = 20) and compute UPPER_BOUND as
10.pow(PAD_WIDTH) - 1 using BigInteger operations, and rename UpperBound to
UPPER_BOUND to follow constant-style naming.
1 participant
Description
Quick & dirty proof of concept of iOS target (using CMP, without SwiftUI).
Still a lot to do to validate and clean, in particular:
licenses_ios.jsonfollowing recent lib updates onmainbranch.linkDebugFrameworkIosSimulatorArm64withoutxcodebuild, still roughly the same duration)Checklist