Backport GH Actions to rhel8-branch #371
Conversation
|
Looks like you're getting the same pylint-only errors as #343, so I think it's good? |
| - python-version: 3.7 | ||
| tox-env: py37 | ||
| steps: | ||
| - name: "Clone Repository" |
There was a problem hiding this comment.
You should also rebase the PR on top of the target branch. Something like:
https://github.com/rhinstaller/anaconda/blob/master/.github/workflows/validate-rhel-8.yml#L70
However, you need to get target branch for the given PR which should be doable by https://docs.github.com/en/free-pro-team@latest/actions/reference/environment-variables#default-environment-variables .
There was a problem hiding this comment.
I don't think so. This runs in pull_request, and the clone step gets the right code. IIRC it was pull_request_target where you have to rebase things. https://docs.github.com/en/free-pro-team@latest/actions/reference/events-that-trigger-workflows#pull_request
There was a problem hiding this comment.
No, with pull_request_target you will have the same code as pull_request. However, this is not what I meant. When you look on checkout action you will find:
Only a single commit is fetched by default, for the ref/SHA that triggered the workflow.
What triggered this code is SHA of the PR not rebase of the PR on the target branch.
The point is, that you are testing exact code which you have on PR. However, the target HEAD of the PR is moving, so you are not testing what will be there after the merge will happen but before the merge. This could be problematic and could miss problems especially when the PR in question is old.
There was a problem hiding this comment.
Ah! I see. That's not usually a problem for me, and when it is I just rebase the PR and force-push it to run it against the new HEAD.
I'm not sure if I like the idea of doing that automatically -- when I write the PR I write it against a specific branching point, if the GH test then tests it against a different point that seems unexpected to me. I'll have to think about that.
bcl
force-pushed
the
rhel8-gh-actions
branch
2 times, most recently
from
945433c
to
1e71b5f
Compare
|
Note that this needs a rhbz in order to be merged. I'll open one today and add it to the commits. |
Related: rhbz#1916487
This switches to using actions via tox, it currently runs for python versions: 3.6, 3.7 It uploads the coverage report from the python 3.6 run. Resolves: rhbz#1916487
Related: rhbz#1916487
The switch to setuptools dropped this, breaking script installation. Related: rhbz#1916487
Related: rhbz#1916487
Related: rhbz#1916487
Related: rhbz#1916487
Related: rhbz#1916487 Test class names are split on _ so the leading _ resulted in a '' which caused the test to fail. I am not sure why this wasn't caught by the nosetests.
…found Related: rhbz#1916487
No description provided.