-
Notifications
You must be signed in to change notification settings - Fork 7
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 label checker #185
fix label checker #185
Conversation
@ajschmidt8, regarding the Could we do this (in a separate PR) by omitting PRs whose titles begin with [RELEASE], rather than by the authorship of GPUTester, as is currently done? The logic would also be updated to omit PRs authored by |
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.
Does the GPUtester
open up any other PRs? If not, this PR looks good to me.
If so, we'll probably want to amend the logic to exclude PRs from both GPUtester
and rapids-bot
.
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.
After reading your comment, we should probably exclude both GPUtester
and rapids-bot
PRs
Looking at the author is safer than looking at the PR title. Otherwise anyone could skip the checks, which is not what we want. |
@ajschmidt8 GPUTester does not open any PRs. It used to though (e.g here, in 2023). These days, the release PRs that come out of code freeze come authored by Ray (e.g here) I guess my question would be, seeing that release PRs are no longer authored by GPUTester but Ray, even if we include an exclusion of PRs from GPUTester, release PRs (which seems to be what we have intended to exclude in the past) will NOT be excluded from checks. |
In that case, these changes are probably fine. I wasn't aware that Ray started opening release PRs now. I don't think there's any good way to exclude release PRs so we can just punt on that problem for now. |
PR updates the label checker plugin to correctly skip Forward Merger PRs.