bvandeusen
785ebdba59
Reminders have been settable since the editor landed and have never once gone off. The board showed them overdue in red, which tells you what you already know by the time you are looking at the board. **AlarmManager, not WorkManager.** The background sync is right to be on WorkManager — nobody minds whether it runs at 3:05 or 3:19. A reminder minds very much. WorkManager's periodic floor is fifteen minutes and it batches into maintenance windows, so "remind me at 09:00" would routinely arrive at 09:14, which is not a reminder, it is a rebuke. **One alarm, not one per reminder.** Only the earliest future reminder is ever scheduled; when it fires, everything due is announced and the next is scheduled. A hundred reminders cost one alarm, and there is no incremental bookkeeping to drift — `Reminders.refresh` recomputes the whole picture from the store, and is called from everywhere anything could have changed: an edit, a foreground, a background sync, boot, and an app update. Boot and MY_PACKAGE_REPLACED both matter and both are easy to forget. Pending alarms survive neither, and this app updates by APK from its own server, so without that receiver a phone would silently stop reminding anyone of anything after a restart — the worst kind of failure, because nothing appears wrong. **Neither permission is treated as a prerequisite.** SCHEDULE_EXACT_ALARM, not USE_EXACT_ALARM: the latter is granted at install with no prompt and is reserved for apps whose whole purpose is an alarm clock or a calendar, which this is not. Refusing the former costs precision, not the feature — it falls back to an inexact alarm, because a reminder a few minutes late beats no reminder. POST_NOTIFICATIONS is asked for on the first launch where a reminder actually exists, never at launch on an empty board. Android gives an app essentially one chance at that dialog, and spending it before the person has any idea what this app would send them is spending it on nothing. For anyone who refuses, or who turns notifications off later in system settings, the Reminders view carries a standing notice with a button to the right screen — a feature that silently does nothing is worse than one that is plainly absent. **A first run adopts overdue reminders silently.** The storm case is linking a server and pulling months of history; a hundred notifications the moment someone signs in is a good way to have the feature turned off before it is ever useful. After that, a missed reminder is announced up to a day late — the web uses fifteen minutes because an open tab has been polling every forty-five seconds, but a phone can be switched off all night. Done and Snooze act from the shade without opening the app. The dedupe key is note id plus remind_at, the same one the web store uses, so snoozing produces a new occurrence rather than one already dealt with. Tapping a notification opens that note. The extra is CONSUMED when read: the Activity keeps the intent it was launched with, so without that, rotating the phone would replay it and reopen a note the person had already closed. `Reminders` split into scheduling policy and `ReminderNotification` rendering after detekt counted fourteen functions in one object — it was right, they answer different questions and change for different reasons. `ForegroundTransitions` moves to the ui package; the reminder notice needs it to re-read a permission the person may have just changed in a system screen this app cannot observe. Known gap, pre-existing and shared with every surface: `complete_reminder` in the core clears a reminder without advancing recurrence — its own comment says so. So tapping Done on a daily reminder ends it rather than moving it to tomorrow. Not changed here because it is core behaviour the desktop and web also have, but notifications make it much easier to hit, and it should be next.
75 lines
3.3 KiB
YAML
75 lines
3.3 KiB
YAML
# Per-rule overrides layered on top of detekt's defaults
|
|
# (`--build-upon-default-config` on the CLI invocation in the Android lane).
|
|
#
|
|
# The pre-2.0 `build:` top-level was removed; failure is controlled by the CLI's
|
|
# exit code instead.
|
|
|
|
naming:
|
|
# Composables conventionally use PascalCase function names. Matches every
|
|
# mainstream Compose codebase, and mirrors the ktlint exemption in
|
|
# android/.editorconfig — the two tools have to agree or one of them is always
|
|
# wrong.
|
|
FunctionNaming:
|
|
ignoreAnnotated:
|
|
- "Composable"
|
|
|
|
style:
|
|
MagicNumber:
|
|
ignoreAnnotated:
|
|
- "Composable"
|
|
# Colour literals and dp constants are declared as named properties, which is
|
|
# exactly the "define it as a well-named constant" the rule asks for — the
|
|
# number simply appears in the declaration itself. Flagging
|
|
# `private val Brand = Color(0xFFF5C518)` would demand a constant holding the
|
|
# constant.
|
|
ignorePropertyDeclaration: true
|
|
|
|
complexity:
|
|
# Compose breaks the PREMISE of both rules below, not just their thresholds.
|
|
#
|
|
# * LongParameterList assumes a long list means an over-general function. A
|
|
# composable's parameters ARE its UI contract — Material's own TextField
|
|
# takes twenty — and collapsing them into a parameter object makes the call
|
|
# site worse, not better, because named arguments are what keep a Compose
|
|
# tree readable.
|
|
# * LongMethod assumes length tracks branching. A composable's length tracks
|
|
# how many ELEMENTS are on the screen; a full-screen editor with a title, a
|
|
# body, a checklist, labels and a reminder row is long because it renders
|
|
# five things, and cutting it into five one-call wrappers would add
|
|
# indirection without removing a single decision.
|
|
#
|
|
# Scoped to @Composable rather than disabled: on ordinary functions both rules
|
|
# are right, and one of them still fires below (see BoardViewModel).
|
|
LongParameterList:
|
|
ignoreAnnotated:
|
|
- "Composable"
|
|
LongMethod:
|
|
ignoreAnnotated:
|
|
- "Composable"
|
|
|
|
exceptions:
|
|
TooGenericExceptionCaught:
|
|
# Catching broadly is DELIBERATE in these two places, and each site says so.
|
|
#
|
|
# * the ViewModel — a note that fails to save must become a visible error
|
|
# banner, never a crash. Narrowing this would mean an unanticipated
|
|
# failure takes the app down instead of being reported, which is strictly
|
|
# worse for the user.
|
|
# * the Application — the store failing to open is the one thing that must
|
|
# still let the app start, so it can explain itself.
|
|
# * the background Worker — it runs with nobody present, so an escaping
|
|
# exception is a crash report for a job the person never asked for. Every
|
|
# realistic failure there (no route, server down, token rotating) has the
|
|
# same right answer, which is Result.retry().
|
|
# * the reminder BroadcastReceiver — same argument, one step worse: it can be
|
|
# woken at 3am by an alarm or by BOOT_COMPLETED, and every path inside it
|
|
# has already logged its own failure by the time this catches anything.
|
|
#
|
|
# Scoped to those paths rather than disabled globally: elsewhere the rule is
|
|
# right and still applies.
|
|
excludes:
|
|
- "**/ui/**"
|
|
- "**/ThoughtSyncApplication.kt"
|
|
- "**/SyncWorker.kt"
|
|
- "**/ReminderReceiver.kt"
|