Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Track forgotten nominations #2553

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

shtlrs
Copy link
Member

@shtlrs shtlrs commented Apr 20, 2023

closes #2226

The idea is to fetch active nominations that are more than 2 weeks old, and create admin issues for them in Github so that staff will do the follow up.

As soon as a vote is tracked, we add a "馃帿" reaction (which can change) to it to make sure we don't create duplicate issues for it, instead of keeping track in another way

@shtlrs shtlrs requested a review from wookie184 as a code owner April 20, 2023 21:34
@shtlrs shtlrs added p: 2 - normal Normal Priority s: needs review Author is waiting for someone to review and approve labels Apr 20, 2023
@Xithrius Xithrius added t: feature New feature or request a: backend Related to internal functionality and utilities (error_handler, logging, security, utils and core) a: recruitment Related to recruitment: (talentpool) labels May 2, 2023

owner: str = "python-discord"
name: str = "mods"
token: str
Copy link
Member

Choose a reason for hiding this comment

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

This PR throws an error when the github token is missing (when it tries it get it from .env file as defined in constants). Maybe we can try making this optional. We also need to update the .env guide on the site https://www.pythondiscord.com/pages/guides/pydis-guides/contributing/bot/#appendix-full-env-file-options

Suggested change
token: str
token: str = ""

Copy link
Member Author

Choose a reason for hiding this comment

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

Ha ! Yes.

It didn't before because we weren't using pydantic at that time.

Good catch, i will take care of it!

Copy link
Member

@RohanJnr RohanJnr May 17, 2023

Choose a reason for hiding this comment

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

we already have API_KEYS_GITHUB (its under _Keys as github) as optional, is the new token constant any different or does it use a different auth approach ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Github introduced tokens per repo.

So this one will be granular in regards to the repo where tasks will be created

Copy link
Member

Choose a reason for hiding this comment

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

ah okay, cool

Copy link
Member

@vivekashok1221 vivekashok1221 left a comment

Choose a reason for hiding this comment

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

This is purely a code review, will test later.

My "changes requested" are more of an attempt to make sure that I understand what's happening clearly.

@@ -652,6 +663,92 @@ async def _nomination_to_string(self, nomination: Nomination) -> str:

return lines.strip()

@tasks.loop(hours=72)
async def track_forgotten_nominations(self) -> None:
"""Track active nominations who are more than 2 weeks old."""
Copy link
Member

Choose a reason for hiding this comment

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

I feel like it would be more accurate to use the term active reviews instead of active nominations because active nomination refers to candidates in the talentpool who may or may not be currently undrgoing review in #nominations-voting.

Copy link
Member

Choose a reason for hiding this comment

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

Or at least that's how I heard the phrase "active nominations" being used.


for nomination in nominations:
# We avoid the scenario of this task run & nomination created at the same time
if not nomination.thread_id:
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure I understand the purpose of this statement because it looks like it's taken care of here . Is it here in case this method is used somewhere else in a different scenario?

section = "github_mods_"

owner: str = "python-discord"
name: str = "mods"
Copy link
Member

Choose a reason for hiding this comment

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

I very vaguely remember the discussion reagrding this feature so I would like to double check whether the issue needs to be raised on the python-discord/mods repo or python-discord/admins (?) repo.

Whenever a vote has passed the upvote threshold, the admins add a ticket to the admin tasks tracker** and not python-discord/mods repo (AFAICT) - which is why I'm asking for clarification on which repo stale reviews should be posted.

**I'm not actually sure what the "admin tasks tracker" is, I always assumed it's a bunch of issues raised on python-discord/admins or something, similar to the python-discord/meta repo.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, admin tasks tracker is the admin-tasks repo.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, the issue is to be raised on python-discord/admin-tasks, Chris just confirmed.

@wookie184 wookie184 added s: waiting for author Waiting for author to address a review or respond to a comment and removed s: needs review Author is waiting for someone to review and approve labels Jan 1, 2024
@wookie184 wookie184 removed their request for review March 25, 2024 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a: backend Related to internal functionality and utilities (error_handler, logging, security, utils and core) a: recruitment Related to recruitment: (talentpool) p: 2 - normal Normal Priority s: waiting for author Waiting for author to address a review or respond to a comment t: feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create an admin task when votes are 2 weeks old
5 participants