build(android): upgrade AGP to 8.6.0, Gradle to 8.7 and raise minSdk to 23 #431
Conversation
WalkthroughAdds Facebook OAuth support across app code and platform configs, injects FB_APP_ID/FB_APP_NAME as build-time dart-defines in CI/CD, upgrades Android Gradle/tooling, lowers iOS deployment target, modifies tests/utilities, and removes the entire Windows desktop build/runner and generated plugin registration. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (4 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
android/gradle/wrapper/gradle-wrapper.properties (1)
6-6: AGP 8.6.0 ⇄ Gradle 8.7 pairing looks right; consider switching to -bin.zip.Functionally OK. If you don’t need sources/javadoc in CI, prefer the smaller distribution to speed up downloads.
Apply if desired:
-distributionUrl=https\://services.gradle.org/distributions/gradle-8.7-all.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-8.7-bin.zippubspec.yaml (1)
81-82: Document temporary overrides; plan removal when Dart ≥ 3.10 lands in Flutter stable.Add context so future maintainers know why these pins exist and when to remove them.
dependency_overrides: - build_runner: ^2.4.13 + # TEMP: Keep analyzer < 6.9 due to Dart 3.9 (no macros). Remove when Flutter stable ships Dart ≥ 3.10. + build_runner: ^2.4.13
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
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 (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
.gitignore(1 hunks)android/app/build.gradle(2 hunks)android/gradle/wrapper/gradle-wrapper.properties(1 hunks)android/settings.gradle(1 hunks)pubspec.yaml(3 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
pubspec.yaml
[error] 75-75: trailing spaces
(trailing-spaces)
[error] 114-114: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (6)
.gitignore (1)
24-24: Ignoring .vscode is fine; proceed.Good call to keep editor-specific settings out of VCS.
android/settings.gradle (1)
21-21: No changes needed: Kotlin Gradle Plugin 2.1.10 is officially supported by AGP 8.6.0.pubspec.yaml (1)
63-63: No production imports of mockito detected; safe to move to dev_dependenciesandroid/app/build.gradle (3)
74-76: Lint DSL migration looks correct.lintOptions → lint with same rules is AGP 8.x-compatible.
80-82: Ensure minSdkVersion ≥ 23
Double-check thatflutter.minSdkVersionresolves to at least 23 and satisfies any plugin requirements; if it resolves below 23, explicitly setminSdkVersion = 23to avoid runtime failures.
58-58: Verify Android API level 36 availability on CI
sdkmanager wasn’t found during automated checks; manually confirm that API 36 is installed on your CI image. If it isn’t, revertcompileSdkVersionto 35 until SDK 36 is generally available.
Location: android/app/build.gradle:58
| transparent_image: ^2.0.0 | ||
| url_launcher: ^6.3.2 | ||
| flutter_facebook_auth: ^6.0.4 | ||
| mockito: ^5.4.5 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Move mockito to dev_dependencies and drop the override or align versions.
Mockito is test-only; keeping it under dependencies bloats the runtime graph and conflicts with the override (5.4.4 vs ^5.4.5).
Apply:
flutter_facebook_auth: ^6.0.4
- mockito: ^5.4.5
+
@@
dev_dependencies:
build_runner: ^2.5.4
flutter_lints: ^6.0.0
flutter_test:
sdk: flutter
hive_generator: ^2.0.0
+ mockito: ^5.4.4
@@
dependency_overrides:
build_runner: ^2.4.13
- mockito: 5.4.4📝 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.
| mockito: ^5.4.5 | |
| flutter_facebook_auth: ^6.0.4 | |
| dev_dependencies: | |
| build_runner: ^2.5.4 | |
| flutter_lints: ^6.0.0 | |
| flutter_test: | |
| sdk: flutter | |
| hive_generator: ^2.0.0 | |
| mockito: ^5.4.4 | |
| dependency_overrides: | |
| build_runner: ^2.4.13 |
🤖 Prompt for AI Agents
In pubspec.yaml around line 63, mockito is listed under dependencies with
version ^5.4.5 while an override pins 5.4.4; move mockito from dependencies into
dev_dependencies (since it is test-only) and either remove the
dependency_override for mockito or update it to match ^5.4.5 so the versions
align; ensure pubspec.yaml has no runtime dependency on mockito and that the
dev_dependencies entry uses the intended version constraint.
| - asset: assets/fonts/Poppins/Poppins-Regular.ttf | ||
| - asset: assets/fonts/Poppins/Poppins-Italic.ttf | ||
| style: italic | ||
| style: italic No newline at end of file |
There was a problem hiding this comment.
Add newline at EOF to satisfy linters.
- style: italic
+ style: italic
+📝 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.
| style: italic | |
| style: italic | |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 114-114: no new line character at the end of file
(new-line-at-end-of-file)
🤖 Prompt for AI Agents
In pubspec.yaml around line 114, the file currently ends with "style: italic"
and lacks a trailing newline which fails linters; open the file and add a single
newline character at EOF so the file ends with a blank line after "style:
italic", save and re-run linters.
ShashwatXD
changed the title
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/cd.yml (1)
223-246: Quote dart-define values (spaces/special chars) and fail fast on unset secrets.Unquoted values can split; add
set -euo pipefailand quote FB defines.run: | + set -euo pipefail # Get version code from run number VERSION_CODE=${{ github.run_number }} @@ - flutter build appbundle --release \ + flutter build appbundle --release \ --build-name=${{ env.VERSION }} \ --build-number=$VERSION_CODE \ - --dart-define=FB_APP_ID=$FB_APP_ID \ - --dart-define=FB_APP_NAME=$FB_APP_NAME \ + --dart-define=FB_APP_ID="$FB_APP_ID" \ + --dart-define=FB_APP_NAME="$FB_APP_NAME" \ --dart-define=GITHUB_OAUTH_CLIENT_ID=$GH_OAUTH_CLIENT_ID \ --dart-define=GITHUB_OAUTH_CLIENT_SECRET=$GH_OAUTH_CLIENT_SECRET @@ - flutter build apk --release \ + flutter build apk --release \ --build-name=${{ env.VERSION }} \ --build-number=$VERSION_CODE \ - --dart-define=FB_APP_ID=$FB_APP_ID \ - --dart-define=FB_APP_NAME=$FB_APP_NAME \ + --dart-define=FB_APP_ID="$FB_APP_ID" \ + --dart-define=FB_APP_NAME="$FB_APP_NAME" \ --dart-define=GITHUB_OAUTH_CLIENT_ID=$GH_OAUTH_CLIENT_ID \ --dart-define=GITHUB_OAUTH_CLIENT_SECRET=$GH_OAUTH_CLIENT_SECRETlib/viewmodels/cv_landing_viewmodel.dart (1)
99-107: Await logout to complete before UI updates.
onLogout()isasyncbut isn’t awaited inonLogoutPressed, leading to racy state/snackbar. ChangeonLogout()to returnFuture<void>and await it here.Apply:
- onLogout(); + await onLogout();
♻️ Duplicate comments (3)
pubspec.yaml (3)
75-75: Remove trailing whitespace.- +
114-114: Add newline at end of file.- style: italic + style: italic +
62-63: Move mockito to dev_dependencies.Mockito is a testing library and should be in
dev_dependenciesrather than regulardependencies. This also creates a version conflict with the override at line 82.Apply this fix:
url_launcher: ^6.3.2 flutter_facebook_auth: ^6.0.4 - mockito: ^5.4.5 http: anyThen add to dev_dependencies section:
flutter_test: sdk: flutter hive_generator: ^2.0.0 + mockito: ^5.4.4
🧹 Nitpick comments (15)
android/gradle/wrapper/gradle-wrapper.properties (1)
6-6: Optional: add distributionSha256Sum for supply-chain hardening.Regenerate the wrapper (
./gradlew wrapper --gradle-version 8.7 --distribution-type all) to populatedistributionSha256Sum..github/workflows/cd.yml (1)
239-239: Trim trailing whitespace.yaml-lint flagged trailing spaces on this line.
ios/Runner/Info.plist (1)
57-57: Ensure FACEBOOK_CLIENT_TOKEN is defined or remove the key.If
$(FACEBOOK_CLIENT_TOKEN)isn’t set, the app may ship with a literal or empty token.lib/enums/auth_type.dart (1)
3-9: Adding enum value mid-list can break persisted ordinals; verify storage and consider appending instead.If
AuthTypeis ever persisted/serialized by index, insertingauthTypeValuesyou’re fine; otherwise, append the new value to the end or migrate persisted data.Possible safer reordering:
-enum AuthType { EMAIL, FACEBOOK, GOOGLE, GITHUB } +enum AuthType { EMAIL, GOOGLE, GITHUB, FACEBOOK }lib/viewmodels/cv_landing_viewmodel.dart (2)
46-49: Comment nit: double period.Apply:
- // Perform facebook logout if auth type is facebook.. + // Perform Facebook logout if auth type is facebook.
41-57: Change onLogout signature to Future and capture authType before clearing storage.Capture the original authType in a local variable, then clear
isLoggedIn,currentUser, andtoken, and branch on that saved value for provider-specific logout. Marking the method asFuture<void>aligns with async best practices. Optionally, clearauthTypeafter sign-out to avoid stale state.lib/config/environment_config.dart (1)
31-34: Provide defaults to avoid hard compile-time requirement for dart-defines in local/test builds.Without
defaultValue, some toolchains fail when the define is absent. Match existing pattern if intentional; otherwise add empty defaults.Apply:
- static const String FB_APP_NAME = String.fromEnvironment('FB_APP_NAME'); - static const String FB_APP_ID = String.fromEnvironment('FB_APP_ID'); + static const String FB_APP_NAME = + String.fromEnvironment('FB_APP_NAME', defaultValue: ''); + static const String FB_APP_ID = + String.fromEnvironment('FB_APP_ID', defaultValue: '');Ensure CI/test workflows pass these via --dart-define or rely on these defaults.
test/ui_tests/groups/update_assignment_view_test.dart (2)
118-131: Prefer final for mocks and keep registrations concise.Apply:
- var dialogService = MockDialogService(); + final dialogService = MockDialogService(); locator.registerSingleton<DialogService>(dialogService); @@ - var updateAssignmentViewModel = MockUpdateAssignmentViewModel(); + final updateAssignmentViewModel = MockUpdateAssignmentViewModel(); locator.registerSingleton<UpdateAssignmentViewModel>( updateAssignmentViewModel, );
154-165: Simulate real tap and avoid fixed 5s wait.Use
tester.tapandpumpAndSettleto reduce flakiness and speed up tests.Apply:
- Widget widget = buttonFinder.evaluate().first.widget; - (widget as CVPrimaryButton).onPressed!(); - await tester.pumpAndSettle(); - await tester.pump(const Duration(seconds: 5)); + await tester.tap(buttonFinder); + await tester.pumpAndSettle();lib/services/API/contributors_api.dart (1)
20-27: Maintain compatibility with both list and wrapped payloads; avoid needless encode/decode if possible.Current code assumes a top-level list and does a JSON round-trip. Prefer handling both shapes explicitly; optionally reintroduce a
fromListconverter to parse without stringify.Apply:
- return circuitVerseContributorsFromJson(jsonEncode(_jsonResponse)); + final res = _jsonResponse; + if (res is List) { + return circuitVerseContributorsFromJson(jsonEncode(res)); + } else if (res is Map && res['contributors'] is List) { + return circuitVerseContributorsFromJson(jsonEncode(res['contributors'])); + } else { + throw Failure(Constants.BAD_RESPONSE_FORMAT); + }Optional further improvement (model-level): add a
circuitVerseContributorsFromList(List<dynamic>)to avoidjsonEncode+ parse.Also, confirm
ApiUtils.getsets a GitHub-compatibleUser-Agent; if not, add one toheadersto prevent 403s.lib/viewmodels/authentication/auth_options_viewmodel.dart (1)
35-35: Add null safety check for access token.While the code uses the non-null assertion operator (
!), it would be safer to explicitly handle the case where the access token might be null.- _storage.token = await _userApi.oauthSignup( - accessToken: result.accessToken!.token, - provider: 'facebook', - ); + final accessToken = result.accessToken?.token; + if (accessToken == null) { + throw Failure('Facebook access token is null'); + } + _storage.token = await _userApi.oauthSignup( + accessToken: accessToken, + provider: 'facebook', + );.github/workflows/ci.yml (2)
104-104: Confirm macOS runner pinning for iOS buildsmacos-latest can jump Xcode versions unexpectedly. If you rely on a specific Xcode, pin the image; otherwise keep latest.
Optional pin:
- - platform: macos-latest + - platform: macos-14
171-171: Trim trailing spaces flagged by lintersLines 171, 213, 214, and 218 have trailing spaces. Please remove to satisfy YAMLlint.
Also applies to: 213-214, 218-218
.metadata (1)
16-20: Windows platform entry removed; web added — was this intentional?Workflows still build Windows. If Windows support is still desired, confirm this .metadata change won’t affect future flutter migrate operations.
ios/Runner.xcodeproj/project.pbxproj (1)
394-394: iOS deployment target lowered to 11.0 — recheck pod/App Store requirementsMany modern SDKs require iOS 12+ or 13+. Confirm Cocoapods resolve cleanly and that your distribution target satisfies App Store policy.
If needed, bump:
- IPHONEOS_DEPLOYMENT_TARGET = 11.0; + IPHONEOS_DEPLOYMENT_TARGET = 13.0;Also applies to: 480-480, 530-530
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
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 (3)
ios/Podfile.lockis excluded by!**/*.lockpubspec.lockis excluded by!**/*.lockwindows/runner/resources/app_icon.icois excluded by!**/*.ico
📒 Files selected for processing (38)
.github/workflows/cd.yml(2 hunks).github/workflows/ci.yml(4 hunks).metadata(1 hunks)android/app/build.gradle(5 hunks)android/app/src/main/AndroidManifest.xml(1 hunks)android/gradle/wrapper/gradle-wrapper.properties(1 hunks)android/settings.gradle(1 hunks)ios/Flutter/AppFrameworkInfo.plist(1 hunks)ios/Podfile(1 hunks)ios/Runner.xcodeproj/project.pbxproj(11 hunks)ios/Runner/Info.plist(2 hunks)lib/config/environment_config.dart(1 hunks)lib/enums/auth_type.dart(1 hunks)lib/models/cv_contributors.dart(0 hunks)lib/services/API/contributors_api.dart(2 hunks)lib/ui/views/authentication/components/auth_options_view.dart(2 hunks)lib/viewmodels/authentication/auth_options_viewmodel.dart(2 hunks)lib/viewmodels/cv_landing_viewmodel.dart(2 hunks)pubspec.yaml(4 hunks)test/ui_tests/groups/update_assignment_view_test.dart(2 hunks)test/utils_tests/image_test_utils.dart(0 hunks)windows/.gitignore(0 hunks)windows/CMakeLists.txt(0 hunks)windows/flutter/CMakeLists.txt(0 hunks)windows/flutter/generated_plugin_registrant.cc(0 hunks)windows/flutter/generated_plugin_registrant.h(0 hunks)windows/flutter/generated_plugins.cmake(0 hunks)windows/runner/CMakeLists.txt(0 hunks)windows/runner/Runner.rc(0 hunks)windows/runner/flutter_window.cpp(0 hunks)windows/runner/flutter_window.h(0 hunks)windows/runner/main.cpp(0 hunks)windows/runner/resource.h(0 hunks)windows/runner/runner.exe.manifest(0 hunks)windows/runner/utils.cpp(0 hunks)windows/runner/utils.h(0 hunks)windows/runner/win32_window.cpp(0 hunks)windows/runner/win32_window.h(0 hunks)
💤 Files with no reviewable changes (19)
- test/utils_tests/image_test_utils.dart
- windows/flutter/generated_plugin_registrant.h
- windows/.gitignore
- windows/runner/resource.h
- windows/runner/runner.exe.manifest
- windows/runner/utils.h
- lib/models/cv_contributors.dart
- windows/runner/win32_window.h
- windows/runner/flutter_window.h
- windows/flutter/generated_plugins.cmake
- windows/runner/CMakeLists.txt
- windows/runner/main.cpp
- windows/runner/utils.cpp
- windows/flutter/CMakeLists.txt
- windows/flutter/generated_plugin_registrant.cc
- windows/runner/Runner.rc
- windows/runner/flutter_window.cpp
- windows/CMakeLists.txt
- windows/runner/win32_window.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
- android/settings.gradle
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-17T13:11:01.586Z
Learnt from: JatsuAkaYashvant
PR: CircuitVerse/mobile-app#381
File: test/ui_tests/groups/my_groups_view_test.dart:136-148
Timestamp: 2025-06-17T13:11:01.586Z
Learning: When mocking async methods in Flutter/Dart tests, always use `thenAnswer((_) async {})` or `thenAnswer((_) => Future.value())` instead of `thenReturn(null)` to maintain the async contract and prevent runtime errors when production code awaits the result.
Applied to files:
test/ui_tests/groups/update_assignment_view_test.dart
🪛 YAMLlint (1.37.1)
.github/workflows/cd.yml
[error] 239-239: trailing spaces
(trailing-spaces)
.github/workflows/ci.yml
[error] 171-171: trailing spaces
(trailing-spaces)
[error] 213-213: trailing spaces
(trailing-spaces)
[error] 214-214: trailing spaces
(trailing-spaces)
[error] 218-218: trailing spaces
(trailing-spaces)
pubspec.yaml
[error] 75-75: trailing spaces
(trailing-spaces)
[error] 114-114: no new line character at the end of file
(new-line-at-end-of-file)
🪛 actionlint (1.7.7)
.github/workflows/ci.yml
222-222: input "github-token" is not defined in action "softprops/action-gh-release@v2". available inputs are "append_body", "body", "body_path", "discussion_category_name", "draft", "fail_on_unmatched_files", "files", "generate_release_notes", "make_latest", "name", "prerelease", "preserve_order", "repository", "tag_name", "target_commitish", "token"
(action)
223-223: input "parallel-finished" is not defined in action "softprops/action-gh-release@v2". available inputs are "append_body", "body", "body_path", "discussion_category_name", "draft", "fail_on_unmatched_files", "files", "generate_release_notes", "make_latest", "name", "prerelease", "preserve_order", "repository", "tag_name", "target_commitish", "token"
(action)
🔇 Additional comments (20)
android/gradle/wrapper/gradle-wrapper.properties (1)
6-6: LGTM: Gradle 8.7 matches AGP 8.6.0 requirements.test/ui_tests/groups/update_assignment_view_test.dart (1)
124-124: Confirm popDialog’s return type; use thenAnswer for async methods.If
popDialog()returnsFuture<void>, mock withthenAnswer((_) async {})to preserve async contract (see prior learning).Apply if async:
- when(dialogService.popDialog()).thenReturn(null); + when(dialogService.popDialog()).thenAnswer((_) async {});lib/viewmodels/authentication/auth_options_viewmodel.dart (1)
24-70: LGTM! Well-structured Facebook OAuth implementation.The Facebook authentication flow follows the same pattern as the existing Google and GitHub auth methods, with proper error handling and state management.
lib/ui/views/authentication/components/auth_options_view.dart (2)
34-45: Facebook auth UI implementation looks good.The Facebook authentication UI follows the same pattern as Google and GitHub, maintaining consistency in the codebase.
97-97: Resolved: Facebook icon asset exists
Assetassets/icons/facebook_icon.pngverified present; no changes required.android/app/build.gradle (4)
26-38: Environment variables implementation looks good.The implementation for handling dart-defines and Facebook configuration is well-structured. The URL decoding and map conversion properly handles the CI/CD environment variables.
85-87: CI/CD workflows include FB_APP_ID and FB_APP_NAME via secrets and--dart-define; no action required.
80-81: Verify and document the source of flutter.minSdkVersion
TheminSdkVersion flutter.minSdkVersionvalue is injected by the Flutter Gradle plugin (seeflutter.gradlein your local Flutter SDK underpackages/flutter_tools/gradle/). Please confirm the default integer (typically 16) and add a note in your project’s docs or above this line inandroid/app/build.gradleso it’s clear where the minimum SDK is coming from.
58-58: Confirm compileSdkVersion 36 compatibility
- Android Gradle Plugin 8.6.0 (android/settings.gradle) and Gradle wrapper 8.7 (android/gradle/wrapper/gradle-wrapper.properties) must support API 36.
- Ensure Android SDK Platform 36 is installed on all CI and local machines.
- No warnings from
./gradlew dependencies, but run full CI builds and integration tests to catch any runtime or API issues.android/app/src/main/AndroidManifest.xml (3)
81-99: Facebook configuration added correctly.The Facebook SDK integration in the manifest is properly configured with the required metadata and activities.
86-92: Activities are correctly exported.The Facebook activities are properly marked with
android:exported="true"as required for Android 12+ (API 31+).
84-89: Ensure compileSdkVersion ≥ 34 for flutter_facebook_auth 6.0.4
flutter_facebook_auth 6.0.4 bundles Facebook Android SDK 16.3.0, which supports API 34+. Confirm your android/compileSdkVersion is set to 34 or higher (e.g., 36) and that your Android Gradle Plugin version is compatible. If you hit any compile-time dependency errors, upgrade to a newer flutter_facebook_auth release or adjust your compileSdk/AGP accordingly..github/workflows/ci.yml (3)
85-86: Good hardening: randomized test order with coverageThis helps surface hidden test inter-dependencies. No action needed.
228-228: Preview job depends only on tests — sensibleKeeps preview fast while still guarded by test pass. Looks good.
279-285: Preview artifact handling is clear and scopedShort retention with explicit naming is good hygiene. No changes needed.
.metadata (1)
7-8: Flutter channel/revision bumped to stableLooks fine; just ensure CI runners use the same stable toolchain to avoid “doctor” mismatches.
ios/Runner.xcodeproj/project.pbxproj (4)
16-16: Pods_Runner.framework reference updates look consistentReferences and build phase wiring align. No action needed.
Also applies to: 57-58, 127-134
208-221: Flutter “Thin Binary” phase uses embed_and_thin — OKMatches the typical Flutter iOS setup. No changes needed.
222-235: Flutter build phase script is correctInvoking xcode_backend.sh build in the main run script is expected. Good.
264-287: Heavy [CP] Embed Pods Frameworks list — verify iOS minimum target compatibilityGiven these pods (e.g., Facebook SDK) often require iOS ≥ 12 or 13, double-check they support IPHONEOS_DEPLOYMENT_TARGET below.
Also applies to: 289-317
| FB_APP_ID: ${{ secrets.FB_APP_ID }} | ||
| FB_APP_NAME: ${{ secrets.FB_APP_NAME }} | ||
| GH_OAUTH_CLIENT_ID: ${{ secrets.GH_OAUTH_CLIENT_ID }} | ||
| GH_OAUTH_CLIENT_SECRET: ${{ secrets.GH_OAUTH_CLIENT_SECRET }} | ||
| run: | | ||
| flutter build apk --debug \ | ||
| --dart-define=FB_APP_ID=$FB_APP_ID \ | ||
| --dart-define=FB_APP_NAME=$FB_APP_NAME \ | ||
| --dart-define=GITHUB_OAUTH_CLIENT_ID=$GH_OAUTH_CLIENT_ID \ | ||
| --dart-define=GITHUB_OAUTH_CLIENT_SECRET=$GH_OAUTH_CLIENT_SECRET |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Quote dart-define values to handle spaces (Android)
FB_APP_NAME can contain spaces; unquoted expansion will break argument parsing.
Apply:
flutter build apk --debug \
- --dart-define=FB_APP_ID=$FB_APP_ID \
- --dart-define=FB_APP_NAME=$FB_APP_NAME \
+ --dart-define=FB_APP_ID="$FB_APP_ID" \
+ --dart-define=FB_APP_NAME="$FB_APP_NAME" \
--dart-define=GITHUB_OAUTH_CLIENT_ID=$GH_OAUTH_CLIENT_ID \
--dart-define=GITHUB_OAUTH_CLIENT_SECRET=$GH_OAUTH_CLIENT_SECRET📝 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.
| FB_APP_ID: ${{ secrets.FB_APP_ID }} | |
| FB_APP_NAME: ${{ secrets.FB_APP_NAME }} | |
| GH_OAUTH_CLIENT_ID: ${{ secrets.GH_OAUTH_CLIENT_ID }} | |
| GH_OAUTH_CLIENT_SECRET: ${{ secrets.GH_OAUTH_CLIENT_SECRET }} | |
| run: | | |
| flutter build apk --debug \ | |
| --dart-define=FB_APP_ID=$FB_APP_ID \ | |
| --dart-define=FB_APP_NAME=$FB_APP_NAME \ | |
| --dart-define=GITHUB_OAUTH_CLIENT_ID=$GH_OAUTH_CLIENT_ID \ | |
| --dart-define=GITHUB_OAUTH_CLIENT_SECRET=$GH_OAUTH_CLIENT_SECRET | |
| run: | | |
| flutter build apk --debug \ | |
| --dart-define=FB_APP_ID="$FB_APP_ID" \ | |
| --dart-define=FB_APP_NAME="$FB_APP_NAME" \ | |
| --dart-define=GITHUB_OAUTH_CLIENT_ID=$GH_OAUTH_CLIENT_ID \ | |
| --dart-define=GITHUB_OAUTH_CLIENT_SECRET=$GH_OAUTH_CLIENT_SECRET |
🤖 Prompt for AI Agents
In .github/workflows/ci.yml around lines 160 to 169, the --dart-define
expansions are unquoted so values containing spaces (e.g., FB_APP_NAME) will be
split into separate arguments; update the flutter build command to quote the
expanded env vars (for example --dart-define=FB_APP_NAME="$FB_APP_NAME") — apply
quoting consistently to all --dart-define entries and ensure any embedded quotes
are escaped so the shell passes each value as a single argument.
| # iOS Build Validation | ||
| - name: Build iOS Debug (No Code Signing) | ||
| if: matrix.build-type == 'ios' | ||
| env: | ||
| FB_APP_ID: ${{ secrets.FB_APP_ID }} | ||
| FB_APP_NAME: ${{ secrets.FB_APP_NAME }} | ||
| GH_OAUTH_CLIENT_ID: ${{ secrets.GH_OAUTH_CLIENT_ID }} | ||
| GH_OAUTH_CLIENT_SECRET: ${{ secrets.GH_OAUTH_CLIENT_SECRET }} | ||
| run: | | ||
| flutter build ios --debug --no-codesign --simulator \ | ||
| cd ios && pod install && cd .. | ||
| flutter build ios --debug --no-codesign \ | ||
| --dart-define=FB_APP_ID=$FB_APP_ID \ | ||
| --dart-define=FB_APP_NAME=$FB_APP_NAME \ | ||
| --dart-define=GITHUB_OAUTH_CLIENT_ID=$GH_OAUTH_CLIENT_ID \ | ||
| --dart-define=GITHUB_OAUTH_CLIENT_SECRET=$GH_OAUTH_CLIENT_SECRET |
There was a problem hiding this comment.
🛠️ Refactor suggestion
iOS: trim trailing spaces and quote dart-define values
- Line 171 has trailing spaces; please trim.
- Quote dart-define values to avoid breaking when names include spaces.
- # iOS Build Validation
+ # iOS Build Validation
...
- flutter build ios --debug --no-codesign \
- --dart-define=FB_APP_ID=$FB_APP_ID \
- --dart-define=FB_APP_NAME=$FB_APP_NAME \
+ flutter build ios --debug --no-codesign \
+ --dart-define=FB_APP_ID="$FB_APP_ID" \
+ --dart-define=FB_APP_NAME="$FB_APP_NAME" \
--dart-define=GITHUB_OAUTH_CLIENT_ID=$GH_OAUTH_CLIENT_ID \
--dart-define=GITHUB_OAUTH_CLIENT_SECRET=$GH_OAUTH_CLIENT_SECRET📝 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.
| # iOS Build Validation | |
| - name: Build iOS Debug (No Code Signing) | |
| if: matrix.build-type == 'ios' | |
| env: | |
| FB_APP_ID: ${{ secrets.FB_APP_ID }} | |
| FB_APP_NAME: ${{ secrets.FB_APP_NAME }} | |
| GH_OAUTH_CLIENT_ID: ${{ secrets.GH_OAUTH_CLIENT_ID }} | |
| GH_OAUTH_CLIENT_SECRET: ${{ secrets.GH_OAUTH_CLIENT_SECRET }} | |
| run: | | |
| flutter build ios --debug --no-codesign --simulator \ | |
| cd ios && pod install && cd .. | |
| flutter build ios --debug --no-codesign \ | |
| --dart-define=FB_APP_ID=$FB_APP_ID \ | |
| --dart-define=FB_APP_NAME=$FB_APP_NAME \ | |
| --dart-define=GITHUB_OAUTH_CLIENT_ID=$GH_OAUTH_CLIENT_ID \ | |
| --dart-define=GITHUB_OAUTH_CLIENT_SECRET=$GH_OAUTH_CLIENT_SECRET | |
| # iOS Build Validation | |
| - name: Build iOS Debug (No Code Signing) | |
| if: matrix.build-type == 'ios' | |
| env: | |
| FB_APP_ID: ${{ secrets.FB_APP_ID }} | |
| FB_APP_NAME: ${{ secrets.FB_APP_NAME }} | |
| GH_OAUTH_CLIENT_ID: ${{ secrets.GH_OAUTH_CLIENT_ID }} | |
| GH_OAUTH_CLIENT_SECRET: ${{ secrets.GH_OAUTH_CLIENT_SECRET }} | |
| run: | | |
| cd ios && pod install && cd .. | |
| flutter build ios --debug --no-codesign \ | |
| --dart-define=FB_APP_ID="$FB_APP_ID" \ | |
| --dart-define=FB_APP_NAME="$FB_APP_NAME" \ | |
| --dart-define=GITHUB_OAUTH_CLIENT_ID=$GH_OAUTH_CLIENT_ID \ | |
| --dart-define=GITHUB_OAUTH_CLIENT_SECRET=$GH_OAUTH_CLIENT_SECRET |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 171-171: trailing spaces
(trailing-spaces)
🤖 Prompt for AI Agents
.github/workflows/ci.yml around lines 171 to 185: line 171 contains trailing
spaces that should be removed, and the --dart-define values passed to flutter
build ios need to be wrapped in quotes so values containing spaces are
preserved; remove the trailing whitespace on the "Build iOS Debug (No Code
Signing)" step line and change each --dart-define assignment to use quoted
values (e.g., --dart-define=FB_APP_ID="..." etc.), ensuring secrets are still
referenced via the environment variables.
| FB_APP_ID: ${{ secrets.FB_APP_ID }} | ||
| FB_APP_NAME: ${{ secrets.FB_APP_NAME }} | ||
| GH_OAUTH_CLIENT_ID: ${{ secrets.GH_OAUTH_CLIENT_ID }} | ||
| GH_OAUTH_CLIENT_SECRET: ${{ secrets.GH_OAUTH_CLIENT_SECRET }} | ||
| run: flutter build windows --debug --dart-define=GITHUB_OAUTH_CLIENT_ID=$env:GH_OAUTH_CLIENT_ID --dart-define=GITHUB_OAUTH_CLIENT_SECRET=$env:GH_OAUTH_CLIENT_SECRET | ||
| run: | | ||
| flutter build windows --debug \ | ||
| --dart-define=FB_APP_ID=$FB_APP_ID \ | ||
| --dart-define=FB_APP_NAME=$FB_APP_NAME \ | ||
| --dart-define=GITHUB_OAUTH_CLIENT_ID=$GH_OAUTH_CLIENT_ID \ | ||
| --dart-define=GITHUB_OAUTH_CLIENT_SECRET=$GH_OAUTH_CLIENT_SECRET |
There was a problem hiding this comment.
Windows step will fail: uses Bash syntax on PowerShell
On windows-latest, default shell is pwsh. Backslashes for line continuation and $VAR expansions won’t work.
Two viable fixes; pick one:
Option A (use Bash):
- name: Build Windows Debug
if: matrix.build-type == 'windows'
env:
FB_APP_ID: ${{ secrets.FB_APP_ID }}
FB_APP_NAME: ${{ secrets.FB_APP_NAME }}
GH_OAUTH_CLIENT_ID: ${{ secrets.GH_OAUTH_CLIENT_ID }}
GH_OAUTH_CLIENT_SECRET: ${{ secrets.GH_OAUTH_CLIENT_SECRET }}
+ shell: bash
run: |
flutter build windows --debug \
- --dart-define=FB_APP_ID=$FB_APP_ID \
- --dart-define=FB_APP_NAME=$FB_APP_NAME \
- --dart-define=GITHUB_OAUTH_CLIENT_ID=$GH_OAUTH_CLIENT_ID \
- --dart-define=GITHUB_OAUTH_CLIENT_SECRET=$GH_OAUTH_CLIENT_SECRET
+ --dart-define=FB_APP_ID="$FB_APP_ID" \
+ --dart-define=FB_APP_NAME="$FB_APP_NAME" \
+ --dart-define=GITHUB_OAUTH_CLIENT_ID="$GH_OAUTH_CLIENT_ID" \
+ --dart-define=GITHUB_OAUTH_CLIENT_SECRET="$GH_OAUTH_CLIENT_SECRET"Option B (keep pwsh):
- run: |
- flutter build windows --debug \
- --dart-define=FB_APP_ID=$FB_APP_ID \
- --dart-define=FB_APP_NAME=$FB_APP_NAME \
- --dart-define=GITHUB_OAUTH_CLIENT_ID=$GH_OAUTH_CLIENT_ID \
- --dart-define=GITHUB_OAUTH_CLIENT_SECRET=$GH_OAUTH_CLIENT_SECRET
+ shell: pwsh
+ run: >
+ flutter build windows --debug
+ --dart-define=FB_APP_ID="$env:FB_APP_ID"
+ --dart-define=FB_APP_NAME="$env:FB_APP_NAME"
+ --dart-define=GITHUB_OAUTH_CLIENT_ID="$env:GH_OAUTH_CLIENT_ID"
+ --dart-define=GITHUB_OAUTH_CLIENT_SECRET="$env:GH_OAUTH_CLIENT_SECRET"🤖 Prompt for AI Agents
.github/workflows/ci.yml lines 191-200: the Windows step uses Bash
line-continuation and $VAR expansion but the runner default shell on
windows-latest is PowerShell, so the step will fail; either set the step to use
Bash (e.g., add a shell: bash directive so existing backslash continuations and
$VAR expansions work) or keep PowerShell and convert the script to valid pwsh
syntax (use PowerShell line-continuation or single-line commands and reference
environment variables as $env:FB_APP_ID, $env:FB_APP_NAME,
$env:GH_OAUTH_CLIENT_ID, $env:GH_OAUTH_CLIENT_SECRET and wrap each --dart-define
value appropriately).
| coverage-finish: | ||
| name: Finalize Coverage | ||
| needs: [test, build-validation] | ||
| needs: test | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| contents: write | ||
| steps: | ||
| - name: Finalize Coveralls Reporting | ||
| uses: coverallsapp/github-action@v2 | ||
| with: | ||
| github-token: ${{ secrets.GITHUB_TOKEN }} | ||
| parallel-finished: true | ||
| coverage-reporter-version: v0.6.15 | ||
| fail-on-error: false | ||
|
|
||
| - name: Download APK Artifact | ||
| uses: actions/download-artifact@v5 | ||
| with: | ||
| name: android-apk | ||
| path: build/app/outputs/flutter-apk/ | ||
| name: android-apk-${{ github.run_number }} | ||
| path: build/app/outputs/apk/release/ | ||
|
|
||
| - name: Get Current Date | ||
| id: date | ||
| run: echo "date=$(date +'%Y-%m-%d')" >> $GITHUB_OUTPUT | ||
|
|
||
| - name: Checkout Repository (for tagging) | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 | ||
| ref: ${{ github.sha }} | ||
|
|
||
| - name: Create Release Tag | ||
| if: github.event_name == 'push' && (github.ref == 'refs/heads/master' || github.ref == 'refs/heads/main' || github.ref == 'refs/heads/develop') | ||
| run: | | ||
| git config --global user.email "actions@github.com" | ||
| git config --global user.name "GitHub Actions" | ||
| git tag -a v${{ github.run_number }} -m "Release v${{ github.run_number }}" | ||
| git push origin v${{ github.run_number }} | ||
|
|
||
|
|
||
| - name: Create Release | ||
| if: github.event_name == 'push' && (github.ref == 'refs/heads/master' || github.ref == 'refs/heads/main' || github.ref == 'refs/heads/develop') | ||
| uses: softprops/action-gh-release@v2 | ||
| with: | ||
| token: ${{ secrets.GITHUB_TOKEN }} | ||
| tag_name: v${{ github.run_number }} | ||
| files: build/app/outputs/flutter-apk/app-debug.apk | ||
| github-token: ${{ secrets.GITHUB_TOKEN }} | ||
| parallel-finished: true | ||
|
|
There was a problem hiding this comment.
Coveralls finalize and release step misconfig; APK download is orphaned
- parallel-finished belongs to Coveralls finalize, not softprops/action-gh-release.
- softprops expects token (not github-token).
- “Download APK Artifact” fails: no step produces android-apk-${{ github.run_number }} in this workflow.
Fix and simplify:
coverage-finish:
name: Finalize Coverage
needs: test
runs-on: ubuntu-latest
steps:
- name: Finalize Coveralls Reporting
uses: coverallsapp/github-action@v2
+ with:
+ github-token: ${{ secrets.GITHUB_TOKEN }}
+ parallel-finished: true
- - name: Download APK Artifact
- uses: actions/download-artifact@v5
- with:
- name: android-apk-${{ github.run_number }}
- path: build/app/outputs/apk/release/
-
- name: Get Current Date
id: date
run: echo "date=$(date +'%Y-%m-%d')" >> $GITHUB_OUTPUT
-
- name: Create Release
uses: softprops/action-gh-release@v2
with:
- github-token: ${{ secrets.GITHUB_TOKEN }}
- parallel-finished: true
+ token: ${{ secrets.GITHUB_TOKEN }}If releases are handled in .github/workflows/cd.yml, consider deleting the “Create Release” step here entirely.
📝 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.
| coverage-finish: | |
| name: Finalize Coverage | |
| needs: [test, build-validation] | |
| needs: test | |
| runs-on: ubuntu-latest | |
| permissions: | |
| contents: write | |
| steps: | |
| - name: Finalize Coveralls Reporting | |
| uses: coverallsapp/github-action@v2 | |
| with: | |
| github-token: ${{ secrets.GITHUB_TOKEN }} | |
| parallel-finished: true | |
| coverage-reporter-version: v0.6.15 | |
| fail-on-error: false | |
| - name: Download APK Artifact | |
| uses: actions/download-artifact@v5 | |
| with: | |
| name: android-apk | |
| path: build/app/outputs/flutter-apk/ | |
| name: android-apk-${{ github.run_number }} | |
| path: build/app/outputs/apk/release/ | |
| - name: Get Current Date | |
| id: date | |
| run: echo "date=$(date +'%Y-%m-%d')" >> $GITHUB_OUTPUT | |
| - name: Checkout Repository (for tagging) | |
| uses: actions/checkout@v4 | |
| with: | |
| fetch-depth: 0 | |
| ref: ${{ github.sha }} | |
| - name: Create Release Tag | |
| if: github.event_name == 'push' && (github.ref == 'refs/heads/master' || github.ref == 'refs/heads/main' || github.ref == 'refs/heads/develop') | |
| run: | | |
| git config --global user.email "actions@github.com" | |
| git config --global user.name "GitHub Actions" | |
| git tag -a v${{ github.run_number }} -m "Release v${{ github.run_number }}" | |
| git push origin v${{ github.run_number }} | |
| - name: Create Release | |
| if: github.event_name == 'push' && (github.ref == 'refs/heads/master' || github.ref == 'refs/heads/main' || github.ref == 'refs/heads/develop') | |
| uses: softprops/action-gh-release@v2 | |
| with: | |
| token: ${{ secrets.GITHUB_TOKEN }} | |
| tag_name: v${{ github.run_number }} | |
| files: build/app/outputs/flutter-apk/app-debug.apk | |
| github-token: ${{ secrets.GITHUB_TOKEN }} | |
| parallel-finished: true | |
| coverage-finish: | |
| name: Finalize Coverage | |
| needs: test | |
| runs-on: ubuntu-latest | |
| steps: | |
| - name: Finalize Coveralls Reporting | |
| uses: coverallsapp/github-action@v2 | |
| with: | |
| github-token: ${{ secrets.GITHUB_TOKEN }} | |
| parallel-finished: true | |
| - name: Get Current Date | |
| id: date | |
| run: echo "date=$(date +'%Y-%m-%d')" >> $GITHUB_OUTPUT | |
| - name: Create Release | |
| uses: softprops/action-gh-release@v2 | |
| with: | |
| token: ${{ secrets.GITHUB_TOKEN }} |
🧰 Tools
🪛 actionlint (1.7.7)
222-222: input "github-token" is not defined in action "softprops/action-gh-release@v2". available inputs are "append_body", "body", "body_path", "discussion_category_name", "draft", "fail_on_unmatched_files", "files", "generate_release_notes", "make_latest", "name", "prerelease", "preserve_order", "repository", "tag_name", "target_commitish", "token"
(action)
223-223: input "parallel-finished" is not defined in action "softprops/action-gh-release@v2". available inputs are "append_body", "body", "body_path", "discussion_category_name", "draft", "fail_on_unmatched_files", "files", "generate_release_notes", "make_latest", "name", "prerelease", "preserve_order", "repository", "tag_name", "target_commitish", "token"
(action)
🪛 YAMLlint (1.37.1)
[error] 213-213: trailing spaces
(trailing-spaces)
[error] 214-214: trailing spaces
(trailing-spaces)
[error] 218-218: trailing spaces
(trailing-spaces)
| - name: Build Preview APK | ||
| env: | ||
| FB_APP_ID: ${{ secrets.FB_APP_ID }} | ||
| FB_APP_NAME: ${{ secrets.FB_APP_NAME }} | ||
| GH_OAUTH_CLIENT_ID: ${{ secrets.GH_OAUTH_CLIENT_ID }} | ||
| GH_OAUTH_CLIENT_SECRET: ${{ secrets.GH_OAUTH_CLIENT_SECRET }} | ||
| run: | | ||
| flutter clean | ||
| flutter pub get | ||
| flutter gen-l10n | ||
| flutter packages pub run build_runner build --delete-conflicting-outputs | ||
| flutter build apk --release \ | ||
| --build-number=${{ github.run_number }} \ | ||
| --dart-define=FB_APP_ID=$FB_APP_ID \ | ||
| --dart-define=FB_APP_NAME=$FB_APP_NAME \ | ||
| --dart-define=GITHUB_OAUTH_CLIENT_ID=$GH_OAUTH_CLIENT_ID \ | ||
| --dart-define=GITHUB_OAUTH_CLIENT_SECRET=$GH_OAUTH_CLIENT_SECRET | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Quote dart-define values in preview build
Same reasoning: names with spaces break argument parsing.
flutter build apk --release \
--build-number=${{ github.run_number }} \
- --dart-define=FB_APP_ID=$FB_APP_ID \
- --dart-define=FB_APP_NAME=$FB_APP_NAME \
- --dart-define=GITHUB_OAUTH_CLIENT_ID=$GH_OAUTH_CLIENT_ID \
- --dart-define=GITHUB_OAUTH_CLIENT_SECRET=$GH_OAUTH_CLIENT_SECRET
+ --dart-define=FB_APP_ID="$FB_APP_ID" \
+ --dart-define=FB_APP_NAME="$FB_APP_NAME" \
+ --dart-define=GITHUB_OAUTH_CLIENT_ID="$GH_OAUTH_CLIENT_ID" \
+ --dart-define=GITHUB_OAUTH_CLIENT_SECRET="$GH_OAUTH_CLIENT_SECRET"📝 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.
| - name: Build Preview APK | |
| env: | |
| FB_APP_ID: ${{ secrets.FB_APP_ID }} | |
| FB_APP_NAME: ${{ secrets.FB_APP_NAME }} | |
| GH_OAUTH_CLIENT_ID: ${{ secrets.GH_OAUTH_CLIENT_ID }} | |
| GH_OAUTH_CLIENT_SECRET: ${{ secrets.GH_OAUTH_CLIENT_SECRET }} | |
| run: | | |
| flutter clean | |
| flutter pub get | |
| flutter gen-l10n | |
| flutter packages pub run build_runner build --delete-conflicting-outputs | |
| flutter build apk --release \ | |
| --build-number=${{ github.run_number }} \ | |
| --dart-define=FB_APP_ID=$FB_APP_ID \ | |
| --dart-define=FB_APP_NAME=$FB_APP_NAME \ | |
| --dart-define=GITHUB_OAUTH_CLIENT_ID=$GH_OAUTH_CLIENT_ID \ | |
| --dart-define=GITHUB_OAUTH_CLIENT_SECRET=$GH_OAUTH_CLIENT_SECRET | |
| - name: Build Preview APK | |
| env: | |
| FB_APP_ID: ${{ secrets.FB_APP_ID }} | |
| FB_APP_NAME: ${{ secrets.FB_APP_NAME }} | |
| GH_OAUTH_CLIENT_ID: ${{ secrets.GH_OAUTH_CLIENT_ID }} | |
| GH_OAUTH_CLIENT_SECRET: ${{ secrets.GH_OAUTH_CLIENT_SECRET }} | |
| run: | | |
| flutter clean | |
| flutter pub get | |
| flutter gen-l10n | |
| flutter packages pub run build_runner build --delete-conflicting-outputs | |
| flutter build apk --release \ | |
| --build-number=${{ github.run_number }} \ | |
| --dart-define=FB_APP_ID="$FB_APP_ID" \ | |
| --dart-define=FB_APP_NAME="$FB_APP_NAME" \ | |
| --dart-define=GITHUB_OAUTH_CLIENT_ID="$GH_OAUTH_CLIENT_ID" \ | |
| --dart-define=GITHUB_OAUTH_CLIENT_SECRET="$GH_OAUTH_CLIENT_SECRET" |
🤖 Prompt for AI Agents
.github/workflows/ci.yml around lines 261 to 278: the flutter build apk
--dart-define flags pass values that may contain spaces (e.g., FB_APP_NAME)
unquoted which breaks argument parsing; update each --dart-define to wrap the
key=value in quotes (for example --dart-define="FB_APP_NAME=$FB_APP_NAME") for
all four defines so the shell treats the entire assignment as a single argument
and preserves spaces/special characters.
| <dict> | ||
| <key>CFBundleURLSchemes</key> | ||
| <array> | ||
| <string>FB_LOGIN_PROTOCOL_SCHEME</string> | ||
| </array> | ||
| </dict> |
There was a problem hiding this comment.
Fix Facebook URL scheme: must be fb$(FB_APP_ID), not a literal placeholder.
Using a literal FB_LOGIN_PROTOCOL_SCHEME breaks login. Use a build setting so Xcode expands it.
- <dict>
- <key>CFBundleURLSchemes</key>
- <array>
- <string>FB_LOGIN_PROTOCOL_SCHEME</string>
- </array>
- </dict>
+ <dict>
+ <key>CFBundleURLSchemes</key>
+ <array>
+ <string>fb$(FB_APP_ID)</string>
+ </array>
+ </dict>📝 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.
| <dict> | |
| <key>CFBundleURLSchemes</key> | |
| <array> | |
| <string>FB_LOGIN_PROTOCOL_SCHEME</string> | |
| </array> | |
| </dict> | |
| <dict> | |
| <key>CFBundleURLSchemes</key> | |
| <array> | |
| <string>fb$(FB_APP_ID)</string> | |
| </array> | |
| </dict> |
🤖 Prompt for AI Agents
In ios/Runner/Info.plist around lines 29-34 the CFBundleURLSchemes entry uses
the literal placeholder "FB_LOGIN_PROTOCOL_SCHEME" which prevents Xcode from
expanding your Facebook app id; replace the literal with the build-variable
expansion so the scheme reads fb$(FB_APP_ID) and ensure the FB_APP_ID build
setting is defined in the target's Build Settings (or in xcconfig) so Xcode can
substitute the actual app id at build time.
| <key>FacebookAppID</key> | ||
| <string>FB_APP_ID</string> | ||
| <key>FacebookDisplayName</key> | ||
| <string>CircuitVerse</string> | ||
| <key>FacebookClientToken</key> | ||
| <string>$(FACEBOOK_CLIENT_TOKEN)</string> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Use Xcode build-setting expansion for FB keys (dart-define won’t touch Info.plist).
Literal strings won’t be replaced at build time; switch to $(VAR) and ensure the vars exist in project/xcconfig.
- <key>FacebookAppID</key>
- <string>FB_APP_ID</string>
- <key>FacebookDisplayName</key>
- <string>CircuitVerse</string>
+ <key>FacebookAppID</key>
+ <string>$(FB_APP_ID)</string>
+ <key>FacebookDisplayName</key>
+ <string>$(FB_APP_NAME)</string>Verification:
- Define FB_APP_ID and FB_APP_NAME in Runner build settings or xcconfig (Debug/Release). Example in
ios/Flutter/Release.xcconfig:
FB_APP_ID=1234567890
FB_APP_NAME=CircuitVerse
🏁 Script executed:
#!/bin/bash
# Locate all xcconfig files in the iOS project
find ios -type f -name "*.xcconfig"
# Search xcconfig files for FB_APP_ID definition
grep -R "FB_APP_ID" -n ios
# Search xcconfig files for FACEBOOK_CLIENT_TOKEN definition
grep -R "FACEBOOK_CLIENT_TOKEN" -n iosLength of output: 280
🏁 Script executed:
#!/bin/bash
echo "=== ios/Flutter/Debug.xcconfig ==="
sed -n '1,200p' ios/Flutter/Debug.xcconfig
echo ""
echo "=== ios/Flutter/Release.xcconfig ==="
sed -n '1,200p' ios/Flutter/Release.xcconfigLength of output: 474
🏁 Script executed:
#!/bin/bash
# Print the beginning of Generated.xcconfig to check existing variable definitions
sed -n '1,200p' ios/Flutter/Generated.xcconfigLength of output: 120
Replace hardcoded Facebook keys in Info.plist with Xcode build‐setting variables
Info.plist (lines 52–57) must use
| @@ -1,3 +1,4 @@ | |||
| import 'package:flutter_facebook_auth/flutter_facebook_auth.dart'; | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Inconsistency with PR objectives.
The PR objectives state this is for upgrading Gradle/AGP, but this file adds Facebook authentication functionality. This appears to be outside the stated scope of the PR.
🏁 Script executed:
#!/bin/bash
# Check if Facebook auth was part of a previous commit or PR
git log --oneline -n 20 | grep -i "facebook\|fb" || echo "No recent Facebook-related commits found"
# Check if there are other Facebook-related changes in the PR
fd -e dart -e java -e kt -e swift -e m | xargs rg -l "facebook\|FacebookAuth" 2>/dev/null | head -10Length of output: 266
PR scope mismatch: move Facebook auth changes to a separate PR
This PR is described as upgrading Gradle/AGP but adds a Facebook authentication import in lib/viewmodels/authentication/auth_options_viewmodel.dart; please remove or isolate this change into its own PR or update the PR description to reflect the added functionality.
🤖 Prompt for AI Agents
In lib/viewmodels/authentication/auth_options_viewmodel.dart around line 1, the
import for flutter_facebook_auth was added but this PR is supposed to only
upgrade Gradle/AGP; either remove the Facebook import and any related code
changes from this branch so the PR only contains build system changes, or move
the Facebook auth change into a separate feature PR (or update this PR
description to explicitly include the Facebook auth addition). If you remove it:
delete the import and any unreachable/unused references, run analyzer/format,
and ensure no other files in this PR depend on flutter_facebook_auth; if you
move it: revert this file to the pre-change state in this branch and create a
new PR containing the Facebook auth import and its associated code and tests.
|
Hey @ShashwatXD , good work, just that the minSDK version is not set properly, please update the root gradle file and explicitly set the number. |
Done @hardik17771 , please review |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
android/app/build.gradle (1)
26-38: Fix parsing of Flutter dart-defines (use Base64, support URL-decoded fallback).Flutter passes dart-defines to Gradle as Base64-encoded key=value pairs. Current URL decoding will mis-parse and break values containing “=” or special chars.
-def environmentVariables = [ - FB_APP_NAME: 'FB_APP_NAME', - FB_APP_ID: 'FB_APP_ID', -]; - -if (project.hasProperty('dart-defines')) { - environmentVariables = environmentVariables + project.property('dart-defines') - .split(',') - .collectEntries { entry -> - def pair = URLDecoder.decode(entry).split('=') - [(pair.first()): pair.last()] - } -} +def environmentVariables = [ + FB_APP_NAME: 'FB_APP_NAME', + FB_APP_ID : 'FB_APP_ID', +] + +if (project.hasProperty('dart-defines')) { + def decoded = project.property('dart-defines') + .split(',') + .collectEntries { entry -> + // Flutter supplies base64-encoded "KEY=VALUE" entries. + def raw + try { + raw = new String(java.util.Base64.decoder.decode(entry)) + } catch (IllegalArgumentException ignore) { + // Fallback for legacy/hand-passed URL-encoded values. + raw = URLDecoder.decode(entry, 'UTF-8') + } + def idx = raw.indexOf('=') + idx > 0 ? [(raw.substring(0, idx)): raw.substring(idx + 1)] : [:] + } + environmentVariables += decoded +}
♻️ Duplicate comments (4)
pubspec.yaml (4)
28-28: Delete stale commented dependency.Remove the redundant
# flutter_facebook_auth: ^5.0.6to avoid confusion now that v6.0.4 is in use.- # flutter_facebook_auth: ^5.0.6
62-63: Move mockito to dev_dependencies and align versions with overrides.Mockito is test-only and the override (5.4.4) conflicts with dependency (^5.4.5). Shift it to dev_dependencies and drop the override to keep resolution stable.
flutter_facebook_auth: ^6.0.4 - mockito: ^5.4.5 + @@ dev_dependencies: build_runner: ^2.5.4 flutter_lints: ^6.0.0 flutter_test: sdk: flutter - hive_generator: ^2.0.0 + hive_generator: ^2.0.0 + mockito: 5.4.4 @@ dependency_overrides: - build_runner: ^2.4.13 - mockito: 5.4.4 + build_runner: ^2.4.13Run to ensure Mockito isn’t referenced from runtime code:
#!/bin/bash rg -nP --type dart -g '!test/**' -C2 '\bpackage:mockito/'Also applies to: 69-76, 80-81
74-74: Trim trailing whitespace.YAML linters flag this line.
- hive_generator: ^2.0.0 + hive_generator: ^2.0.0
113-113: Add newline at EOF.Satisfies YAML/newline linters.
- style: italic + style: italic +
🧹 Nitpick comments (2)
pubspec.yaml (1)
80-81: Document and time-box the overrides.If these overrides are required for Dart 3.9 analyzer/macro constraints, add a comment with the removal condition (e.g., “remove once Flutter stable ships Dart ≥3.10”). Otherwise, drop them.
dependency_overrides: - build_runner: ^2.4.13 + # TEMP: Keep pub get working on Dart 3.9; remove when Flutter stable includes Dart ≥3.10. + build_runner: ^2.4.13 - mockito: 5.4.4 + # Prefer pinning under dev_dependencies and removing this override if possible. + # mockito: 5.4.4android/app/build.gradle (1)
85-87: Don’t override app_name via resValue; guard Facebook IDs.Overriding
app_namefrom CI can unintentionally rename the app. Keepapp_namein strings.xml and set FB values defensively to avoid leaking placeholders into builds.- resValue "string", "app_name", environmentVariables.FB_APP_NAME - resValue "string", "facebook_app_id", environmentVariables.FB_APP_ID - resValue "string", "fb_login_protocol_scheme", "fb" + environmentVariables.FB_APP_ID + // Keep app_name in res/values/strings.xml to avoid accidental renames. + // Provide Facebook values with safe fallbacks. + resValue "string", "facebook_app_id", (environmentVariables.FB_APP_ID ?: "") + resValue "string", "fb_login_protocol_scheme", + (environmentVariables.FB_APP_ID ? "fb${environmentVariables.FB_APP_ID}" : "")If the manifest requires these strings unconditionally, ensure CI always sets FB_APP_ID; otherwise consider using manifestPlaceholders instead of resValue.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
android/app/build.gradle(5 hunks)pubspec.yaml(4 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
pubspec.yaml
[error] 74-74: trailing spaces
(trailing-spaces)
[error] 113-113: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (5)
android/app/build.gradle (5)
15-15: No-op change.Nothing to review here.
74-76: LGTM: lint DSL update.Using the newer
lint {}block is correct for AGP 8.x.
80-82: Min/target SDK update looks fine.minSdkVersion 23 matches the PR goal; targetSdk 34 aligns with Play requirements.
If there’s still feedback about “root Gradle” minSdk, point reviewers to this module-level setting (the authoritative place in Flutter apps).
129-129: No-op change.Nothing to review here.
58-58: Confirm compileSdkVersion 36 is available in CI & local dev.Location: android/app/build.gradle:58
Avoid red builds from missing SDK platforms; install if needed. Sandbox verification failed (sdkmanager/cmdline-tools not present). Run on CI/dev and confirm success:yes | "${ANDROID_HOME:-$ANDROID_SDK_ROOT}"/cmdline-tools/latest/bin/sdkmanager "platforms;android-36" "build-tools;35.0.0"
|
Hey @ShashwatXD , I saw you removed windows desktop support which is not needed just to get a dependency work. Also you've added facebook metadata and facebook auth implementation injection but we are currently in process of removing depricated facebook packages. |
|
You can raise different PRs for these issues as this PR should be focused on upgrading gradle and minSdk versions |
Alright @hardik17771, I only made those changes due to build errors. Will keep the next PR limited to just Gradle and minSdk upgrade. |
You can surely make changes for build errors but we can't remove desktop support for now. |
|
Got it, must have overlooked the desktop part, my bad! |
2 participants
Fixes #423
Describe the changes you have made in this PR -
Upgraded gradle wrapper to 8.7 and android gradle plugin to 8.6.0 along with the required upgrades.
temporary dependency_overrides:
-applied for build_runner and mockito
-Reason: current stable Dart (3.9) cannot resolve analyzer ≥6.9 which depends on macros from Dart 3.10+
-effect: keeps pub get working locally on Dart 3.9
NOTE: overrides are only a stopgap and can be removed once Flutter stable ships Dart 3.10+
Summary by CodeRabbit
New Features
Chores