-
-
Notifications
You must be signed in to change notification settings - Fork 468
Add notifications field to the User class #383
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThe User class now retains the full legacy data and exposes a new notifications flag initialized from the legacy payload. Class diagram for updated User class with notifications fieldclassDiagram
class User {
- _client: Client
- _legacy: dict
id: str
created_at: str
notifications: bool
...
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughAdds a new public boolean attribute notifications to User, sourced from legacy data, and stores the entire legacy dict in a new internal field _legacy. Updates the User class docstring accordingly. No other methods or control flow are changed. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
twikit/user.py (3)
87-88: Clarify semantics; consider Optional when API omits this fieldLegacy often omits
notifications. Treating absence asFalsecan mislead consumers.Apply this docstring tweak:
- notifications : :class:`bool` - Indicates if notifications were enabled for this user. + notifications : Optional[:class:`bool`] + Whether notifications are enabled for this user relative to the authenticating account. None if not provided by the API.
94-94: Defensive copy for_legacyto avoid accidental external mutationStore a shallow copy; annotate precisely.
- self._legacy: dict = legacy + self._legacy: dict[str, Any] = legacy.copy()Add missing import:
-from typing import TYPE_CHECKING, Literal +from typing import TYPE_CHECKING, Literal, Any
129-129: Preserve “unknown” vs “false” in notifications
After searching for all.notificationsreferences, the only occurrence is its initialization in twikit/user.py (line 129), so there are no downstream boolean‐only assumptions that would break:• twikit/user.py:129 – drop the default
Falseand widen the annotation to allowNonefor “not provided by API.”Recommended diff:
- self.notifications: bool = legacy.get('notifications', False) + self.notifications: bool | None = legacy.get('notifications')
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
twikit/user.py(2 hunks)
ditto.
Summary by Sourcery
Add a notifications boolean field to the User class and retain legacy data for future use
New Features:
Enhancements:
Summary by CodeRabbit