build: use pull_request_target to start Jenkins

[mmarchini]

GitHub recently introduced a new Pull Request event for Actions with
access to Secrets and GITHUB_TOKEN with write access. Therefore, we
don't need to use the scheduler event to start Jenkins anymore.

Ref: https://github.blog/2020-08-03-github-actions-improvements-for-fork-and-pull-request-workflows

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

[mmarchini]

Not entirely sure this will work for existing PRs without rebase though. At least when I tested on node-auto-test I had to rebase for the PR to have proper access, might need to specify the base branch explicitly on checkout if that's the case. I'll run more tests on another repo before landing this.

[phillipj]

LooksReallyGoodToMe!

[mmarchini]

Ok, there's a huge downside with this approach: the Action will run on the base commit of the Pull Request, not the base Branch, therefore:

1. If the Action is changed on the base branch but the PR was still pointing to an older version, the older version will run
2. If an Action is added with this event, it will only work for new PRs (assuming the branch was created after the Action was added). For existing PRs, the PR needs to be rebased

Since we have 271 open PRs, rebasing everything every time we change an Action is unfeasible. Therefore we might not be able to use this event for most tasks.

[richardlau]

Ok, there's a huge downside with this approach: the Action will run on the base commit of the Pull Request, not the base Branch, therefore:

1. If the Action is changed on the base branch but the PR was still pointing to an older version, the older version will run
2. If an Action is added with this event, it will only work for new PRs (assuming the branch was created after the Action was added). For existing PRs, the PR needs to be rebased

Since we have 271 open PRs, rebasing everything every time we change an Action is unfeasible. Therefore we might not be able to use this event for most tasks.

It won’t solve the already opened PR case, but could we do the rebasing we do in the Jenkins CI jobs? I guess we’d have to keep the action steps to a minimum and encapsulate the logic in e.g. shell scripts.

[mmarchini]

The Action will not trigger on existing PRs.

[mmarchini]

Seems like the issue_comment event does what we need: it has access to secrets and it always runs on the default branch. The difference is we would use a comment like /jenkins instead of adding a label. One advantage of this is that we can add other commands later (/jenkins v8, /jenkins citgm).

We could also keep both: label using the schedule event and comment using the issue_comment.

What do y'all like more:

• 🚀 label
• 😄 comment
• ❤️ both
• 😕 indifferent

[cjihrig]

Can we please avoid using comments since they trigger GitHub notifications.

[mmarchini]

@cjihrig good point, but starting the CI generates a notification anyway, so in this case it's probably not a problem?

[cjihrig]

but starting the CI generates a notification anyway, so in this case it's probably not a problem?

It makes the problem worse for people receiving emails. FWIW, I've asked in the past to make the bot less noisy as well.

[mmarchini]

Closing for now then, will reopen once nodejs/github-bot#272 lands since that should workaround all the limitations we have with current pull_request and pull_request_target events.