Skip to content

Add a !tp get_review command to get the nomination text without posting it#1503

Merged
Akarys42 merged 6 commits into
mainfrom
tp-get_review-command
Apr 11, 2021
Merged

Add a !tp get_review command to get the nomination text without posting it#1503
Akarys42 merged 6 commits into
mainfrom
tp-get_review-command

Conversation

@Akarys42
Copy link
Copy Markdown
Contributor

@Akarys42 Akarys42 commented Apr 6, 2021

This can be used when information should be added to the post, or someone wants to review the user.

Approved by our nomination overlord @wookie184.

@Akarys42 Akarys42 added p: 1 - high High Priority t: enhancement Changes or improvements to existing features a: recruitment Related to recruitment: (talentpool) labels Apr 6, 2021
@Akarys42 Akarys42 requested a review from wookie184 as a code owner April 6, 2021 09:26
…ng it

This can be used when an information should be added to the post, or someone wants to review the user.
@Akarys42 Akarys42 force-pushed the tp-get_review-command branch from 8276f8e to 2aa7e74 Compare April 6, 2021 09:29
Copy link
Copy Markdown
Contributor

@wookie184 wookie184 left a comment

Choose a reason for hiding this comment

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

Just a small thing.

Also, Discord doesn't seem to display the emojis properly in the file preview, at least for me, I get something like this:

image

The actual file when downloaded seems fine though, seems it could be a discord bug? I guess we could make it display with the :thumbsup: format, but I don't really mind leaving it as-is.

Comment thread bot/exts/recruitment/talentpool/_cog.py
Akarys42 added 2 commits April 6, 2021 14:48
This caused unicode errors in Discord attachment previews.
@Akarys42 Akarys42 requested a review from wookie184 April 6, 2021 12:51
Copy link
Copy Markdown
Contributor

@ToxicKidz ToxicKidz left a comment

Choose a reason for hiding this comment

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

Mostly looks good, one small suggestion.

Comment thread bot/exts/recruitment/talentpool/_review.py Outdated
Co-authored-by: ToxicKidz <78174417+ToxicKidz@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@wookie184 wookie184 left a comment

Choose a reason for hiding this comment

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

Might make sense for line 273 in _review.py, to be changed to use :eyes: instead of 👀 since you've changed the other ones, but this looks good to me, thanks!

@Akarys42
Copy link
Copy Markdown
Contributor Author

Akarys42 commented Apr 6, 2021

Might make sense for line 273 in _review.py, to be changed to use :eyes: instead of 👀 since you've changed the other ones, but this looks good to me, thanks!

To be honest, if we were to do that they should be Unicode points because Git messes up encoding sometimes. But we can't use the emoji names everwhere, it will fail in some places. I don't mind switching everything though.

EDIT: Alright, I see why this one should be :eyes:, will change it.

Copy link
Copy Markdown
Contributor

@ToxicKidz ToxicKidz left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Copy link
Copy Markdown
Contributor

@Numerlor Numerlor left a comment

Choose a reason for hiding this comment

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

Some of the sentences sent to the channel are missing periods but that includes some of the string from he previous revisions - feel free to fix them if you think it's worth it.

L205 in the _cog module is still using the emoji version of a checkmark

Comment thread bot/exts/recruitment/talentpool/_cog.py Outdated
if not await self.reviewer.mark_reviewed(ctx, user_id):
return
await ctx.send(f"✅ The user with ID `{user_id}` was marked as reviewed.")
await ctx.send(f"\u2705 The user with ID `{user_id}` was marked as reviewed.")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The unicode escape is not very clear, the bot has a Emojis.check_mark constant for this emoji

Comment thread bot/exts/recruitment/talentpool/_cog.py Outdated

await self.reviewer.post_review(user_id, update_database=False)
await ctx.message.add_reaction("")
await ctx.message.add_reaction("\u2705")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same as above

nomination = self._pool.watched_users[user_id]
if nomination["reviewed"]:
await ctx.send("❌ This nomination was already reviewed, but here's a cookie 🍪")
await ctx.send("❌ This nomination was already reviewed, but here's a cookie :cookie:")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should use Emojis.cross_mark or :x: if we're taking out emojis out of the code.
Same situation on L303

log.trace(f"Posting the review of {user_id}")
message = (await self._bulk_send(channel, review))[-1]
if seen_emoji:
for reaction in (seen_emoji, "\U0001f44d", "\U0001f44e"):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think using "\N{THUMBS UP SIGN}" and "\N{THUMBS DOWN SIGN}" is a bit clearer for the emojis

@Akarys42 Akarys42 force-pushed the tp-get_review-command branch from 78f9fa7 to e97c8cd Compare April 11, 2021 13:55
@Akarys42 Akarys42 enabled auto-merge April 11, 2021 13:56
Copy link
Copy Markdown
Member

@ChrisLovering ChrisLovering left a comment

Choose a reason for hiding this comment

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

🔥

Copy link
Copy Markdown
Member

@ChrisLovering ChrisLovering left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@Akarys42 Akarys42 merged commit 32ff331 into main Apr 11, 2021
@Akarys42 Akarys42 deleted the tp-get_review-command branch April 11, 2021 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: recruitment Related to recruitment: (talentpool) p: 1 - high High Priority t: enhancement Changes or improvements to existing features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants