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

Fix checking maintainers for teams #1932

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

isuruf
Copy link
Member

@isuruf isuruf commented May 16, 2024

Checklist

  • Added a news entry

@isuruf
Copy link
Member Author

isuruf commented May 16, 2024

cc @BastianZim @dillonroach

@@ -1181,8 +1181,24 @@ def run_conda_forge_specific(meta, recipe_dir, lints, hints):
# Check if all maintainers have either commented or are the PR author
Copy link
Member

@jaimergp jaimergp May 17, 2024

Choose a reason for hiding this comment

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

Seeing this code now, I wonder if we should also get the emoji reactions in the comments Sometimes folks just leave a 👍 as the confirmation.

I think this could work (for the block above):

commenters.update(
    {
        reaction.user.login 
        for comment in issue_comments
        for reaction in issue.get_reactions()
        if reaction.content in ("+1", "heart", "rocket", "hooray")
    }
)

Refs:

Copy link
Member

Choose a reason for hiding this comment

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

I think that's a good idea! My memory is a little vague but I think the problem during implementation was that these features weren't available yet.

Copy link
Member Author

Choose a reason for hiding this comment

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

@jaimergp do you mind sending a PR?

@mfansler
Copy link
Member

This would resolve #1817. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants