[codex] Support signed-out account preferences#3316
Conversation
|
Validation update:
This should be ready for maintainer review. I kept the installable-artifact workflow out of this feature PR; that is tracked separately in #3337. |
veloce
left a comment
veloce
left a comment
There was a problem hiding this comment.
Thanks for this contribution!
I'd like to see another approach for this feature, as explained in one comment.
This is a significant change, and I have not reviewed the PR thoroughly yet (will do after the change). Hope I have not missed something that would prevent doing it, but I don't think so.
| build: | ||
| name: Build ${{ matrix.target }} | ||
| runs-on: macos-latest | ||
| runs-on: macos-26 |
There was a problem hiding this comment.
We've never had issues before, not sure this is a relevant change (not relevant in this PR actually).
| runOnlyForDeploymentPostprocessing = 0; | ||
| shellPath = /bin/sh; | ||
| shellScript = "if [[ \"$PLATFORM_NAME\" == *simulator* ]]; then\n echo \"Skipping Crashlytics upload for simulator build\"\n exit 0\nfi\n\"$PODS_ROOT/FirebaseCrashlytics/upload-symbols\" --flutter-project \"$PROJECT_DIR/firebase_app_id_file.json\" "; | ||
| shellScript = "if [[ \"$PLATFORM_NAME\" == *simulator* ]]; then\n echo \"Skipping Crashlytics upload for simulator build\"\n exit 0\nfi\nUPLOAD_SYMBOLS=\"$PODS_ROOT/FirebaseCrashlytics/upload-symbols\"\nif [[ ! -x \"$UPLOAD_SYMBOLS\" ]]; then\n echo \"Skipping Crashlytics upload; upload-symbols not found at $UPLOAD_SYMBOLS\"\n exit 0\nfi\n\"$UPLOAD_SYMBOLS\" --flutter-project \"$PROJECT_DIR/firebase_app_id_file.json\" "; |
There was a problem hiding this comment.
Have you run that to ensure the iOS build still works?
| ); | ||
|
|
||
| /// Account-style preferences with server values taking precedence when signed in. | ||
| final effectiveAccountPreferencesProvider = FutureProvider<AccountPrefState>((Ref ref) async { |
There was a problem hiding this comment.
Instead of having 3 separate account prefs providers (local, server, effective), which adds extra complexity and code duplication. I'd rather have a single one (the existing AccountPreferences notifier for that matter) that handles the local logic and decides itself which pref to return.
MichaelSpece
force-pushed
the
codex/signed-out-account-preferences
branch
3 times, most recently
from
480778f to
7807aa1
Compare
|
Thanks for the review, @veloce. I updated the PR to use the existing I also removed the temporary workflow and iOS project changes from the final diff; those review threads are now outdated. The PR is rebased on current |
|
Hi @veloce, gentle follow-up now that the PR has been updated to the |
MichaelSpece
force-pushed
the
codex/signed-out-account-preferences
branch
from
7807aa1 to
d65ba3b
Compare
MichaelSpece
force-pushed
the
codex/signed-out-account-preferences
branch
from
d65ba3b to
8fabb93
Compare
2 participants
Summary
Validation
dart run build_runner builddart format lib/src/model/account/account_preferences.dart lib/src/model/game/game_controller.dart lib/src/model/settings/preferences_storage.dart lib/src/view/game/game_body.dart lib/src/view/over_the_board/over_the_board_screen.dart lib/src/view/settings/account_preferences_screen.dart lib/src/view/settings/board_settings_screen.dart lib/src/view/watch/tv_screen.dart test/model/account/account_repository_test.dartflutter analyze lib/src/model/account/account_preferences.dart lib/src/model/game/game_controller.dart lib/src/model/settings/preferences_storage.dart lib/src/view/game/game_body.dart lib/src/view/over_the_board/over_the_board_screen.dart lib/src/view/settings/account_preferences_screen.dart lib/src/view/settings/board_settings_screen.dart lib/src/view/watch/tv_screen.dart test/model/account/account_repository_test.dartflutter test test/model/account/account_repository_test.dartflutter build ios --no-codesign,flutter build appbundle --debug)Closes #3273