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

New rule to detect inactive patch authors #2426

Draft
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

benjaminmah
Copy link
Contributor

@benjaminmah benjaminmah commented Jun 21, 2024

Resolves #1532.

Introduces a new rule to detect and unassign inactive patch authors on Phabricator.

If the patch is not already closed, it will be abandoned.

Checklist

  • Type annotations added to new functions
  • Docs added to functions touched in main classes
  • Dry-run produced the expected results
  • The to-be-announced tag added if this is worth announcing

rev_ids = {rev_id for bug in bugs.values() for rev_id in bug["rev_ids"]}
try:
inactive_authors = self._get_inactive_patch_authors(list(rev_ids))
except Exception as e:
Copy link
Member

Choose a reason for hiding this comment

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

I would avoid using Exception as the error-catching type where possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense! I removed the try-except block here: 243515d. I made sure to do a dry-run and noticed that I did not encounter any errors, so I don't think the exception handler is necessary.

Comment on lines 99 to 100
except Exception as e:
logging.error(f"Failed to abandon patch {rev_id} for bug {bugid}: {e}")
Copy link
Member

Choose a reason for hiding this comment

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

same as #2426 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed here: 3a88716

Comment on lines 122 to 124
except Exception as e:
logging.error(f"Error fetching revisions: {e}")
continue
Copy link
Member

Choose a reason for hiding this comment

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

same as #2426 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed here: 9eabe77

user["unavailable_until"] = phab_user["attachments"][
"availability"
]["until"]
except Exception as e:
Copy link
Member

Choose a reason for hiding this comment

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

same as #2426 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed here: d1b0350

@suhaibmujahid suhaibmujahid added the to-be-announced Improvements to be announced: send to dev-platform, firefox-dev and add to EE newsletter label Jul 8, 2024
@benjaminmah benjaminmah requested a review from marco-c July 29, 2024 13:18
@marco-c marco-c removed their request for review August 5, 2024 16:36
@marco-c
Copy link
Contributor

marco-c commented Aug 5, 2024

@suhaibmujahid I'm leaving this one for you since you started the review already.

"""Bugs with patches authored by inactive patch authors"""

def __init__(self):
super(InactivePatchAuthors, self).__init__()
Copy link
Member

Choose a reason for hiding this comment

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

In Python 3, you do not need to pass any arguments to super():

Suggested change
super(InactivePatchAuthors, self).__init__()
super().__init__()

def __init__(self):
super(InactivePatchAuthors, self).__init__()
self.phab = PhabricatorAPI(utils.get_login_info()["phab_api_key"])
self.user_activity = UserActivity(include_fields=["nick"], phab=self.phab)
Copy link
Member

Choose a reason for hiding this comment

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

Is the nick field needed here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
to-be-announced Improvements to be announced: send to dev-platform, firefox-dev and add to EE newsletter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Detect inactive patch authors on Phabricator
3 participants