Is ifNotFork necessary anymore?

Yes, why wouldn't it be?

It doesn't seem we would ever have a scenario where we would receive a push event and also not immediately bail at the first condition checking the master branch (i.e. I can't see how an error would ever occur if the ifNotFork was not here). I suppose it might still be better to stop it as early as possible to avoid tying it to this specific implementation.

Forks can also have branches named master.

Are we receiving push events for those?

Yes, someone could make a PR from fork/master.

Hm, ok. I think I may have been expecting that this condition would somehow already be verifying that the commit happened on wordpress/gutenberg's master, but I see now that's not the case.

But ifNotFork was implemented for pull request events, and I think would need to be updated to support this.

Notably, payload.pull_request doesn't exist for push events.

https://developer.github.com/v3/activity/events/types/#pushevent

Then it's better to just not use it. I've removed it. No one actually makes PRs from master anyway. We can live without milestones for those edge cases.

Then it's better to just not use it. I've removed it. No one actually makes PRs from master anyway. We can live without milestones for those edge cases.

I don't know how to search for specific examples to share, but I've definitely seen pulls from forked master before. That said, the combination of this and a commit message which matches the (#PR) pattern to get to the point of adding the milestone would be exceedingly rare, yes. And the result would merely be a failing action, nothing too worrisome.

We might want to revisit this in the future to make the logic more durable and to support ifNotFork usage for non-pull_request events (especially given the fact the name is generic enough to easily misinterpret that it ought be supported), but I don't think it's a blocker.