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: getRefs function to handle merge_group events #766

Merged
merged 1 commit into from
Jul 12, 2024

Conversation

louis-bompart
Copy link
Contributor

@louis-bompart louis-bompart commented May 3, 2024

Fixes #456

Essentially, put #456 (comment) in the code.

To the best of my knowledge, this should work, because a merge_group event is a PullRequestEvent I think.

@louis-bompart
Copy link
Contributor Author

👋 @elireisman, sorry for the poke; I was hoping to get a review on this 😶. I think it's a quick win ;)
Feel free to redirect me to someone else if I'm barking at the wrong tree 😄
Thanks!

Copy link
Contributor

@elireisman elireisman left a comment

Choose a reason for hiding this comment

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

This LGTM - and sorry I didn't see this alt. PR sooner, I think I last interacted on the other that was closed out?

I'll also ping our new FR for a second pair of 👀 before we land it cc @pcarlisle

Thanks again for the contribution! ❤️

@louis-bompart
Copy link
Contributor Author

Thanks 🙌
I saw 47a6b35 ❌ on https://github.com/actions/dependency-review-action/actions/runs/8933751781/job/24923072522
I'm assuming this was caused by my branch being a few commits behind main, so I rebased my branch onto main with 92ca1f7

@elireisman elireisman self-requested a review May 14, 2024 17:44
@elireisman
Copy link
Contributor

👋 I think you need to update the PR one more time prior to re-review (see the check-dist entry)

@louis-bompart
Copy link
Contributor Author

👋 I think you need to update the PR one more time prior to re-review (see the check-dist entry)

My bad! I missed the npm run package 😅 before committing. Should be fixed in e7e99e1

@nachtgold
Copy link

Hi @elireisman do you know another possible reviewer than @pcarlisle?

@elireisman
Copy link
Contributor

👋 @nachtgold @louis-bompart apologies - looks like there wasn't time for anyone to circle back to this yet.

Looks like we need one more dist update (npm run package) pushed to the PR to get it updated before we can move forward. Unfortunately, if I check out the branch and do that myself, I won't be able to review the PR either 😄

@louis-bompart
Copy link
Contributor Author

@elireisman :shipit: 😄

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.

Support merge_group trigger
4 participants