Skip to content
This repository was archived by the owner on Apr 27, 2022. It is now read-only.

Conversation

@loxygenK
Copy link
Contributor

短いコードですがReqChangeを授かる自信があります

@loxygenK loxygenK requested a review from isso0424 June 16, 2020 06:31
src/client.py Outdated
async def on_guild_role_create(self, role):
pass

await add_role_to_kawae(load_config(), role)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

なんで毎回load_config()を呼び出してるんですか...?
これメンバ変数にコンフィグを持ったり、roleに対して静的変数として定数を置いておいたほうがよくね



async def add_role_to_kawae(
config: Dict[str, Dict[str, Union[str, int]]],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

必要なconfigだけ受け取ろう

config: Dict[str, Dict[str, Union[str, int]]],
role: discord.Role
):
user: discord.Member = role.guild.get_member(config["add_role"]["user_id"])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

これclassにしてmemberをメンバ変数として持っておきません?


async def on_guild_role_create(self, role):
pass

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

またこの人末尾の改行消してる...

role: discord.Role
):
user: discord.Member = role.guild.get_member(config["add_role"]["user_id"])
notify_channel: discord.TextChannel = role.guild.get_channel(config["text_channel"]["base"])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

なんで一回一回読み込むんですか...?

user: discord.Member = role.guild.get_member(config["add_role"]["user_id"])
notify_channel: discord.TextChannel = role.guild.get_channel(config["text_channel"]["base"])
await user.add_roles(role)
await notify_channel.send(role.mention + "ですが、かわえにも追加しておきました")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

formatしたくない...?

@loxygenK
Copy link
Contributor Author

なんでクラスにしなかったんですかね、たぶんプログラミング苦手だからだと思うんですけど

Copy link
Member

@isso0424 isso0424 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

req

):
await self.kawae_member.add_roles(role_to_add)
await self.notify_channel.send(
"{}ですが、{}にも追加しておきました".format(role_to_add.mention, self.kawae_member.mention)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HARD CODING
hibikiness追加が面倒になるのでやめて


class KawaemonRoleAdder:

def __init__(self, client: discord.Client, kawae_user_id: int, announce_text_channel_id: int):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clientをたらい回しすんな

@loxygenK loxygenK merged commit 4e72114 into newHaracho Jun 16, 2020
@MikuroXina MikuroXina deleted the addrole_to_kawae branch March 22, 2021 14:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants