Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the auth/README.md documentation to reflect API changes in AuthState (transitioning from a sealed class to an abstract class), and adds detailed guidance on MFA configuration, test tags, sign-out behavior, and error handling. The feedback points out that the companion object properties in the documented AuthState class must be initialized to be valid, compilable Kotlin, and provides a code suggestion to fix this.
| @JvmStatic val Idle: Idle | ||
| @JvmStatic val Cancelled: Cancelled | ||
| @JvmStatic val Aborted: Aborted |
There was a problem hiding this comment.
In Kotlin, properties declared inside a companion object must be initialized. To ensure the documented code block is valid and compilable Kotlin, please initialize these companion properties to match their actual implementation in AuthState.kt.
| @JvmStatic val Idle: Idle | |
| @JvmStatic val Cancelled: Cancelled | |
| @JvmStatic val Aborted: Aborted | |
| @JvmStatic val Idle: Idle = Idle() | |
| @JvmStatic val Cancelled: Cancelled = Cancelled() | |
| @JvmStatic val Aborted: Aborted = Aborted() |
0f52224 to
7de7f8d
Compare
7de7f8d to
25d48b9
Compare
auth/README.mddocumented an API that does not exist:AuthStateas a sealed hierarchy of data classes, an MFA section that builds a config object and never passes it anywhere, and an errorwhencovering half the exception types with noelse. Corrected against the source onpre-GA, with the hand-maintained test-tag table replaced by the naming rule the test suite already enforces.Changes
AuthStateblock:abstractnotsealed, nothing is a data class,Error.exceptionis typedException, plusSuccess.reauthenticatedUid,isNotificationand theAuthState.Reauthenticationhierarchy, with a note on what each means for caller code.MFA Configuration now passes
mfaConfigurationtoFirebaseAuthScreen, which is where it goes. It is a screen parameter, not a property ofauthUIConfiguration, and the old example dropped the object it built.Error Handling covers all 21
AuthExceptionsubclasses and ends with anelse, sinceAuthExceptionis not sealed and the missing branches previously failed silently.Test tags: the 32-row table becomes the
fui_<object_snake_case>_<CONSTANT_lowercase>rule, the 13 grouping objects, and a pointer toFirebaseAuthTestTags, citing the reflective assertions inFirebaseAuthTestTagsTest.The stability promise is rescoped from "a value documented here" to every constant on
FirebaseAuthTestTags, bringing the 27 constants the table omitted inside it.Sign Out documents that
signOutclears Google Credential Manager state and the FacebookLoginManagersession for linked providers, with the Facebook step guarded by a classpath probe.The
FirebaseAuthScreenparameter table is marked partial and points at Custom UI with Slots for the seven slot parameters; the drifting "~170 abstractvals" count is dropped rather than re-pinned.Fixes the localization example, which defined
SpanishStringProviderwith no constructor parameters and then calledSpanishStringProvider(context), so the snippet did not compile.Repairs two
AuthExceptionmessages that read "You are are attempting...", the second also missing a word. Own commit, since it is source rather than docs.Validation
Five sweeps re-run on the finished README: symbol resolution, named arguments against each callee, required arguments against each signature, Compose calling context, and anchor/TOC resolution (56 anchors, all resolve, modelling GitHub's duplicate-heading suffixing).
The string change carries
./gradlew :auth:testDebugUnitTest --rerun-tasks: BUILD SUCCESSFUL, 49 tasks executed, 1101 tests, 0 failures or errors. Verified as a real run rather than cached results by diffing the 82 test classes underauth/src/testagainst the 82 inauth/build/test-results, none missing, all XMLs written during this run.Maintainer note: Refs internal CPRN-490