-
Notifications
You must be signed in to change notification settings - Fork 609
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
✨ 提供一个插件好感度限制 #1846
✨ 提供一个插件好感度限制 #1846
Conversation
文件级别更改
提示和命令与 Sourcery 互动
自定义您的体验访问您的 仪表板 以:
获得帮助Original review guide in EnglishReviewer's Guide by SourceryThis pull request introduces a new feature to enforce a minimum '好感度' (impression) limit for plugins. The implementation involves modifications in the authentication process to check a user's impression, updates to the plugin model and settings to include the new field, and utility enhancements to support database schema changes. Class Diagram for Plugin and SignUser Models (with Impression Field)classDiagram
class PluginInfo {
+Boolean is_show
+Float impression
+String module
+Boolean ignore_prompt
...
}
class SignUser {
+String user_id
+Float impression
+async get_user(user_id, platform: String|None)
+async sign(...)
...
}
note for PluginInfo "New field 'impression' added to enforce minimum impression limit"
note for SignUser "Added get_user method to fetch or create SignUser instance"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @HibiKier - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a database migration to populate the
impression
column for existing plugins. - The logic for checking
plugin.impression
is duplicated inauth_plugin
; can this be refactored?
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
if plugin.ignore_prompt: | ||
return False | ||
return self._flmt_s.check(sid) |
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.
suggestion (code-quality): We've found these issues:
- Lift code into else after jump in control flow (
reintroduce-else
) - Replace if statement with if expression (
assign-if-exp
)
if plugin.ignore_prompt: | |
return False | |
return self._flmt_s.check(sid) | |
return False if plugin.ignore_prompt else self._flmt_s.check(sid) |
No description provided.