python-package-template icon indicating copy to clipboard operation
python-package-template copied to clipboard

Improve Pull Request Action behavior

Open jenstroeger opened this issue 3 years ago • 2 comments

The pull-request.yaml Action may not need to trigger the code checks and tests:

https://github.com/jenstroeger/python-package-template/blob/f41b0e6a46061081370253b9ac17a1b62b5c85dc/.github/workflows/pull-request.yaml#L57-L61

if the PR is in “draft” mode — the event payload has a boolean "draft":

build: 
  needs: conventional-commits 
  if: ${{ !github.event.pull_request.draft }}

that we should be able to check. A draft PR by definition is incomplete and may or may not pass checks and tests…

Furthermore, I am tempted to change this

https://github.com/jenstroeger/python-package-template/blob/f41b0e6a46061081370253b9ac17a1b62b5c85dc/.github/workflows/pull-request.yaml#L8-L11

to

 pull_request: 
   branches: 
   - *

to target all PRs regardless of their base branch (i.e. PRs that target other PRs).

We also don’t need to run checks and tests if the PR body discussion has had an entry but there was no code change. In that case, maybe only run cz (because the PR title was changed), for example:

build: 
  needs: conventional-commits 
  if: ${{ github.event.action !== 'edited' }}

Skipping the build job, however, means that no artifacts are produced by that particular Action run, which is probably acceptable.

jenstroeger avatar Sep 21 '22 10:09 jenstroeger

@behnazh I think we can close this one now? Two related issues are merged.

jenstroeger avatar Oct 28 '22 04:10 jenstroeger

It would make sense, methinks, to expand the PR Action to check that a PR does not modify certain files. These “protected” files could be stored in the repository in a dedicated dot-file, e.g. .donottouch.

The intention is to prevent people from modifying these files in downstream repositories, and enforce modifying these files in the parent template repository. Protected files would be e.g. the Makefile or .pre-commit-config.yaml.

(Related issue #370.)

jenstroeger avatar Nov 02 '22 06:11 jenstroeger