Update GitHub Actions to run on pull request events

[samkit-jain]

Context: PR #279 While the PR works, it had one flaw detailed below (apologies for the double PR, my understanding was incomplete).

In this link that I had also shared in the description of #279, there was this statement that to me was a bit confusing.

In order to solve this, we’ve added a new pull_request_target event, which behaves in an almost identical way to the pull_request event with the same set of filters and payload. However, instead of running against the workflow and code from the merge commit, the event runs against the workflow and code from the base of the pull request. This means the workflow is running from a trusted source and is given access to a read/write token as well as secrets enabling the maintainer to safely comment on or label a pull request. This event can be used in combination with the private repository settings as well.

I was unsure what However, instead of running against the workflow and code from the merge commit, the event runs against the workflow and code from the base of the pull request? meant. To get more clarity on this, I raised a support ticket with GitHub and asked the following question (answer also provided):

In the base repo, say, one of the steps is pytest tests/ which runs the tests under the tests folder. There are 10 tests. Now, in the forked repo, I add a new test case taking the total to 11 tests. I raise a PR from my forked repo. Now, when the step would be run, will it run 10 tests (since running on the code from base) or would it run 11 tests (as what the user would expect)?

Our understanding is that for pull_request_target it would run 10 tests. So if you wanted the workflow to run on the merge commit (and run 11 tests), you could use pull_request. The caveat to that is that it might be a security concern as any user can modify workflow files and raise a PR.

While in #253 pull_request was dropped in favour of push to avoid duplicate runs, I guess for it to work with forked repos, we need to add it back. I think using pull_request over pull_request_target should be safe in pdfplumber's case as there are no secrets involved AND if the forked repo PRs don't run on the merge commit, the green tick would be a false positive.

[codecov]

Codecov Report

Merging #282 into stable will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           stable     #282   +/-   ##
=======================================
  Coverage   97.41%   97.41%           
=======================================
  Files          10       10           
  Lines        1160     1160           
=======================================
  Hits         1130     1130           
  Misses         30       30           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c95f4fd...ee69b69. Read the comment docs.

[jsvine]

Thanks for the explanation @samkit-jain! Merging.