Repository navigation
Conversation
- Add reset() to clear all local SDK state including device ID - Deprecate removeUserId() in favor of reset()
|
No actionable comments were generated in the recent review. 🎉 WalkthroughAdds version 0.1.0, introduces Changes
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ced52fda3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
lib/src/core/clix.dart (1)
158-161: Consider documenting concurrency expectations.The docstring could clarify that callers should ensure no other SDK operations are in-flight when calling
reset(), as concurrent operations may encounter null references if they're past the initialization check.📝 Suggested docstring enhancement
/// Resets all local SDK state including device ID. /// /// After calling this method, you must call [initialize] again before using the SDK. /// Use this when a user logs out and you want to start fresh with a new device identity. +/// +/// **Important:** Ensure no other SDK operations are in progress when calling this method. static Future<void> reset() async {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/src/core/clix.dart` around lines 158 - 161, Update the documentation for the reset() method to state concurrency expectations: callers must ensure no other SDK operations are in-flight when calling reset() (i.e., serialize or await any ongoing calls that rely on initialization), because concurrent operations may observe null/cleared state after reset() even if they passed an earlier initialization check; mention that initialize() must be called again before using other SDK methods and recommend using external synchronization or awaiting pending futures around reset()/initialize() to avoid races.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Around line 17-19: The CHANGELOG deprecation guidance and the `@Deprecated`
annotation in clix.dart disagree about the replacement for removeUserId(); align
them to the intended replacement reset(): update the CHANGELOG entry that
currently says "use setUserId(null) instead" to say "use reset() instead" (or
equivalent wording) so it matches the `@Deprecated` message in clix.dart (line
with removeUserId()/@Deprecated) and the PR objective that reset() is the
correct replacement.
In `@lib/src/core/clix.dart`:
- Around line 162-173: The reset() method currently nullifies _shared before
allowing services to perform cleanup; modify reset() to first await
service-level teardowns by calling _shared!._notificationService?.reset() and
_shared!._tokenService?.reset() (and any other service reset methods) and await
their completion, then clear storage keys via
_shared!._storageService?.remove(...), and only after all resets succeed set
_shared = null and reset _isInitializing/_initCompleter; keep the existing catch
that throws ClixError.unknownErrorWithReason on failure, and explicitly decide
whether to clear persistent config fields (project_id, api_key, clix_config) or
leave a comment documenting they persist for re-initialization.
---
Duplicate comments:
In `@lib/src/core/clix.dart`:
- Around line 148-156: The `@Deprecated` message on removeUserId is inconsistent
with the CHANGELOG; change the annotation text on the static method removeUserId
to recommend the correct replacement used in the CHANGELOG (setUserId(null)) or
update the CHANGELOG to match reset(); ensure the deprecation string references
the exact replacement method name (e.g., "Use setUserId(null) instead" or "Use
reset() instead") so the intent in removeUserId, _waitForInitialization, and the
call to _shared!._deviceService!.removeProjectUserId() is clear and consistent.
---
Nitpick comments:
In `@lib/src/core/clix.dart`:
- Around line 158-161: Update the documentation for the reset() method to state
concurrency expectations: callers must ensure no other SDK operations are
in-flight when calling reset() (i.e., serialize or await any ongoing calls that
rely on initialization), because concurrent operations may observe null/cleared
state after reset() even if they passed an earlier initialization check; mention
that initialize() must be called again before using other SDK methods and
recommend using external synchronization or awaiting pending futures around
reset()/initialize() to avoid races.
Summary
Add
reset()method for logout support and deprecateremoveUserId().Changes
reset()method that clears all local SDK state (device ID, session) without server callsremoveUserId()— usereset()insteadSpec
https://docs.google.com/document/d/1ET-KqAFQlODNEm08YDXehzfWuqKZsOhnr4NJG-f0P9k/edit?tab=t.0#bookmark=id.9awmqjhnpii6
Summary by CodeRabbit
New Features
Changed
Deprecated