feat: Flutter app scaffold with dependencies and clean starting point #22
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/flutter-scaffold"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Flutter App — Groundwork (Phase 7 Step 1)
The most basic groundlaying work: Flutter project scaffold with all dependencies from ADR 0017, clean starting point, smoke test.
What's included
pubspec.yaml— all dependencies resolved and verified:redux5.0.0,flutter_redux0.10.0,redux_epics0.15.2dio5.7.0go_router14.6.2freezed_annotation2.4.4,json_annotation4.9.0flutter_secure_storage9.2.2build_runner,freezed,json_serializable,mocktail,very_good_analysis6.0.0analysis_options.yaml— very_good_analysis lints, excluding generated files (*.g.dart,*.freezed.dart)lib/main.dart— stripped default counter app, minimal cleanDoujinManagerAppwith placeholdertest/app_test.dart— smoke test (app renders title)Platforms: Linux, Web, Android only (per ADR 0019 — Windows/macOS/iOS skipped)
Verification
flutter pub get✅flutter analyze→ No issues found ✅flutter test→ 1/1 passed ✅This is intentionally minimal — just the foundation. Next steps: Gallery theme, Redux store setup, folder structure, then feature-by-feature.
Summary
Summary
Coverage
DoujinManager.ApplicationCore - 84.8%
DoujinManager.Infrastructure - 91.6%
pshot
DoujinManager.RestAdapter - 84.2%
E5CB12CEAC5BBBEA65844E1C05F9ADA4DA38B848075C41304FC6A1ABF77__ValidationAttr
ibuteCache
DoujinManager.Server - 20.9%
🤖 Hermes automated review: minor comments
Reviewed diff
5014af3...e4d9d5cb(+1802/-0, 43 files): Flutter project scaffold underapp/with dependencies from ADR 0017, stripped-downmain.dart, and a smoke test. The vast majority of the diff is stockflutter createboilerplate (Android/Linux/Web platform dirs,pubspec.lock, CMake/Gradle configs); the hand-written files arepubspec.yaml,analysis_options.yaml,lib/main.dart,test/app_test.dart, and minor.metadata/README.mdtweaks.ADR 0017 compliance: ✅ All 10 declared dependencies are present with sensible caret constraints (
redux,flutter_redux,redux_epics,dio,go_router,freezed_annotation,json_annotation,flutter_secure_storage,very_good_analysis,mocktail+ dev codegen tools). Platform targets (Linux/Web/Android only) match ADR 0019 — no iOS/Windows/macOS dirs.pubspec.lockis correctly committed (best practice for apps). Static security scan clean (no secrets, injection, eval/exec, pickle, SQL formatting in added lines). No blocking issues — four minor non-blocking notes below.Minor (non-blocking)
1. Android launcher label is lowercase
"doujinmanager"while the app title is"DoujinManager"app/android/app/src/main/AndroidManifest.xml:3—android:label="doujinmanager"The Android home-screen/launcher label is lowercase
doujinmanager, butMaterialApp.titleinlib/main.dart:13isDoujinManager. On Android the launcher icon will readdoujinmanager; everywhere else (web<title>, Linux window title via GTK) it derives from the proper-case title. Consider aligning the manifest label toDoujinManagerfor a consistent brand across platforms.2. Release build signed with debug keys (template default)
app/android/app/build.gradle.kts:37—signingConfig = signingConfigs.getByName("debug")This is the stock
flutter createtemplate with its// TODO: Add your own signing configcomment still present. Fine for a scaffold, but must be replaced before any Play Store / distribution build — debug-signed release APKs are not installable on stock devices and the signing key is publicly predictable. Flagging so it's tracked.3.
very_good_analysispinned at^6.0.0— 4 major versions behind currentapp/pubspec.yaml:31—very_good_analysis: ^6.0.0flutter pub getreportsvery_good_analysis 6.0.0 (10.3.0 available). The pinned release is from mid-2024; 10.x adds newer Dart 3.x lints (e.g. stricterimplicit_call_tearoffs,document_ignores, updatedavoid_print). ADR 0017 doesn't pin a version, so this is just an opportunity — not a defect. Worth bumping in a follow-up once the scaffold lands, since a major-version lint bump later may surface many new warnings at once.4. CI workflow (
ci.yml) does not yet cover the Flutter appThe forgejo-actions coverage comment (#345) reports only the four .NET assemblies (ApplicationCore, Infrastructure, RestAdapter, Server) — the
app/Flutter code has no CI gate. I confirmedflutter analyzeandflutter testpass locally (see Verification below), but untilci.ymlis extended to runflutter analyze/flutter teston theapp/directory, future Flutter changes will merge without automated lint/test coverage. Suggest adding a Flutter job toci.ymlin a near-term follow-up.Verification
e4d9d5cb(line 83.8%, branch 49.8%; generated 06/29 16:42 UTC). Theci.ymltest job posts coverage only on successful .NET build+test, so the backend build+test passed. Backend local build/test skipped per CI-evidence policy.app/Flutter code (CI is absent for this path), so per the CI-absent policy I ran lightweight local checks with Flutter 3.44.4 / Dart 3.12.2:flutter pub get→ ✅ resolved (29 packages have newer incompatible versions, all non-blocking)flutter analyze→ ✅ No issues found! (12.4s)flutter test→ ✅ 1/1 passed (App renders title)git diffbetween base5014af3and heade4d9d5cb.Note: This is a conversation comment, not a formal Forgejo review approval — the current MCP integration cannot create review approval states or inline diff comments.
Automated daily review. I never merge PRs.
Addressed findings in commit
98fe17b.flutter analyze0 issues,flutter test1/1 pass.#1 — Android launcher label ✅ Fixed:
doujinmanager→DoujinManager#2 — Release signing config 📝 Tracked — template default, will be replaced before any distribution build. Not relevant for scaffold.
#3 — very_good_analysis ✅ Bumped:
^6.0.0→^10.0.0(latest). Zero new issues.#4 — Flutter CI job 📝 Tracked — will add a Flutter job to
ci.ymlin a near-term follow-up once there's more app code to gate.🤖 Hermes automated review: no blocking issues found
Follow-up review of commits
98fe17band3f55066(incremental diffe4d9d5cb...3f550660, since the prior review). All actionable findings from the previous review are resolved; one new non-blocking observation on the path-filter change.Prior findings — status
98fe17b:app/android/app/src/main/AndroidManifest.xml:3nowandroid:label="DoujinManager"(matchesMaterialApp.title).very_good_analysis6.0.0 → 10.3.0 ✅ Bumped and verified in98fe17b:app/pubspec.yaml:30nowvery_good_analysis: ^10.0.0,pubspec.lockupdated to 10.3.0. A 4-major-version lint bump is exactly where you'd expect new analyzer violations, so I ran the linter locally —flutter analyze→ "No issues found!" with 10.3.0. Clean.Minor (non-blocking) — new
1. Path filter on
ci.ymlmeans a Flutter-only PR still won't run any CI, and thedocker-publish.ymlpath filter omitsapp/entirely (intended) but also omits.dockerignore.github/workflows/ci.yml:6-9and.github/workflows/docker-publish.yml:6-9The new
pathsfilters correctly scope the backend CI and docker publish jobs tobackend/**+ their own workflow file — this is a good optimization and avoids redundant runs for docs/app-only changes. Two observations:app/directory has zero CI gating (the path filter now makes that explicit rather than implicit). The localflutter analyze/flutter testI ran here is the only check the Flutter code currently gets.docker-publish.ymlpaths listbackend/**,deploy/Dockerfile, and the workflow file, but not.dockerignore. A change to.dockerignorealone won't trigger a container rebuild. Likely fine (.dockerignorechanges are rare and usually paired with Dockerfile changes), just noting the gap.Neither is actionable for this scaffold PR — flagging for when the Flutter CI job is added.
Verification
98fe17bat 18:57 UTC,3f55066after). It is therefore stale for head3f550660. Per the stale-CI policy, I ran lightweight local checks for the changed Flutter code:flutter pub get→ ✅ resolved (28 packages have newer incompatible versions, all non-blocking)flutter analyze(very_good_analysis 10.3.0) → ✅ No issues found!flutter test→ ✅ 1/1 passed (App renders title)git diffbetween prior heade4d9d5cband current head3f550660.Note: This is a conversation comment, not a formal Forgejo review approval — the current MCP integration cannot create review approval states or inline diff comments.
Automated daily review. I never merge PRs.