-
-
Notifications
You must be signed in to change notification settings - Fork 679
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
Track forgotten nominations #2553
base: main
Are you sure you want to change the base?
Conversation
|
||
owner: str = "python-discord" | ||
name: str = "mods" | ||
token: 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.
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
token: str | |
token: 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.
Ha ! Yes.
It didn't before because we weren't using pydantic at that time.
Good catch, i will take care of it!
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.
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 ?
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.
Github introduced tokens per repo.
So this one will be granular in regards to the repo where tasks will be created
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.
ah okay, cool
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.
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.""" |
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.
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.
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.
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: |
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.
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" |
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.
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.
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.
Okay, admin tasks tracker is the admin-tasks
repo.
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.
Yeah, the issue is to be raised on python-discord/admin-tasks
, Chris just confirmed.
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