-
Notifications
You must be signed in to change notification settings - Fork 17
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
Spam PRs can override the original PR's IPR status #172
Comments
I investigated whether it's possible to report statuses on pull requests, instead of on commits. That does not seem possible; GitHub ties statuses very closely to commits. I wonder if the IPR check should have succeeded anyway on the spam PR, since the commits were all authored by @annevk, not by the spammer? That is, we currently check |
Well, unless the commit is verified it can say whatever, right? So the PR author seems like a better source of truth as to who is making the contribution (I think squash will also attribute it to them, git-wise). And even with verified commits we wouldn't want to take a commit if that person didn't actually contribute it to us, I think, even if they signed the agreement. |
Good point. I guess this might be wontfix/unfixable, then. (Although I admit I haven't thought too hard about it.) After all, if you'd rebased and force-pushed to your branch it would have generated a new commit, which would pass the IPR checker. Hmm, another interesting note: I think using /agreement-status would also let you fix the IPR check, even without rebasing. Which would have made that spam PR also pass the IPR check? Eek. Maybe the IPR checker should just fail loudly if there are multiple PRs containing the commit? I dunno, probably not worth it. |
whatwg/xhr#303 is a spam PR where someone forked whatwg/xhr, and then PRed from the
annevk/response-fragments
branch on their fork to the master/main branch on whatwg/xhr.This overwrote the IPR status on the actual PR, whatwg/xhr#200.
This kind of makes sense in that IPR status is reported on a commit, so if it's the same commit hash, I can see how a conflict would arise.
This is similar to #33 but not quite the same.
Unfortunately the "Recent Deliveries" tab for the webhook does not have that spam PR (it's been too long). So I can't inspect the body to figure out a good way of detecting this. If something similar happens, post here and we can capture it. Or, we could reproduce the situation ourselves I suppose.
The text was updated successfully, but these errors were encountered: