-
Notifications
You must be signed in to change notification settings - Fork 43
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
Impl msg/view and made Message compact with msg/view and quoted message. #224
Conversation
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.
还没看 message.py,这个文件改动有点太大了,要稍晚一点看
khl/channel.py
Outdated
@@ -216,7 +216,7 @@ def __init__(self, **kwargs): | |||
self.is_friend: bool = kwargs.get('is_friend') | |||
self.is_blocked: bool = kwargs.get('is_blocked') | |||
self.is_target_blocked: bool = kwargs.get('is_target_blocked') | |||
self.target_info: Dict = kwargs.get('target_info') | |||
self.target_info: User = kwargs.get('target_info') |
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.
换成 User
的话需要构造,因为构造函数内部还有逻辑,不是纯 data class
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.
我保留一下兼容性:
self.target: User = User(...)
self._target_info: Dict = kwargs.get('target_info')
@property
def target_info(self):
warnings.warn("deprecated", ...)
return self._target_info
khl/channel.py
Outdated
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.
可以单独开一个MR改这里的,因为看起来和当前MR主题关联不紧密
khl/command/parser.py
Outdated
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.
推荐单独提MR,拆成小commit过得也很快
khl/client.py
Outdated
req = api.Message.view(msg_id=msg_id) | ||
return PublicMessage(_gate_=self.gate, **(await self.gate.exec_req(req))) | ||
|
||
async def fetch_direct_message(self, chat_code: str, msg_id: str): |
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.
async def fetch_direct_message(self, chat_code: str, msg_id: str): | |
async def fetch_private_message(self, chat_code: str, msg_id: str): |
client层可以屏蔽掉底层API的命名,切换成我们提供的概念,统一性和可理解性更好
khl/command/rule.py
Outdated
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.
可以单独提MR?不过涉及 PublicMessage
,有没有先后次序关系
khl/user.py
Outdated
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.
也可以单独提,不过这个typo改动很小,合并进来也可以
khl/util.py
Outdated
@@ -2,7 +2,7 @@ | |||
|
|||
|
|||
def unpack_id(obj): | |||
"""extract obj's id if not basic data type""" | |||
"""extract objet's id if not basic data type""" |
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.
"""extract objet's id if not basic data type""" | |
"""extract object's id if not basic data type""" |
不过就用 obj 的话能对应参数名,方便理解
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.
obj's
IDE 会划波浪线,不过不该也无所谓
khl/message.py
Outdated
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.
有点太大了,review时间可能会有点长
两个小的MR已提交,修改doc-string的那个不管了。 |
client.py
Client.fetch_message(...)
Client.fetch_direct_message(...)
message.py
更改为
其他