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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PR review reminders skips PRs if they have any update within the urgency period #1984

Closed
sarayourfriend opened this issue May 2, 2023 · 1 comment · Fixed by #2027
Closed
Assignees
Labels
💻 aspect: code Concerns the software code in the repository 🛠 goal: fix Bug fix 🟧 priority: high Stalls work on the project or its dependents 🧱 stack: catalog Related to the catalog and Airflow DAGs 🧱 stack: mgmt Related to repo management and automations

Comments

@sarayourfriend
Copy link
Contributor

Description

If a PR is low urgency and unreviewed for 4 days and then the author pushes a rebase, the PR's "urgency" clock will get reset back to 0.

updated_at = parse_gh_date(pr["updated_at"])
today = datetime.datetime.now()
urgency = pr_urgency(pr)
if urgency is None:
return None
days = days_without_weekends(today, updated_at)

Additional context

I think we should change this to ping since the last request for review. If a PR isn't ready for review yet, the author should draft it.

I've had a lot of PRs get skipped over for reminders because I am diligent in keeping PRs rebased and updated while waiting for reviews. The reminders are effectively useless in the current configuration except for PRs that are totally ignored. With our recent discussions about PR review timeliness and having too many PRs open, I think this change will help make the reminders more useful for our current goals.

Marked high because these reminders are meant to help us review PRs in a timely manner and the current approach is not achieving that. If it's too noisy after this change, we can make further refinements, perhaps looking for specific types of updates to index against, rather than the undraft/open date.

@sarayourfriend sarayourfriend added 🟧 priority: high Stalls work on the project or its dependents 🛠 goal: fix Bug fix 💻 aspect: code Concerns the software code in the repository 🧱 stack: catalog Related to the catalog and Airflow DAGs 🧱 stack: mgmt Related to repo management and automations labels May 2, 2023
@sarayourfriend sarayourfriend self-assigned this May 2, 2023
@obulat
Copy link
Contributor

obulat commented May 3, 2023

Great catch! I haven't seen the PR review reminders on my PRs (even the ones that are more than 2 weeks old) in a long time because I regularly rebase them :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💻 aspect: code Concerns the software code in the repository 🛠 goal: fix Bug fix 🟧 priority: high Stalls work on the project or its dependents 🧱 stack: catalog Related to the catalog and Airflow DAGs 🧱 stack: mgmt Related to repo management and automations
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants