Updates to github workflows for PDO #504
Conversation
*EXPERIMENTAL* -- new github workflows will only be visible and runnable when they are attached to the main branch. this commit will enable further testing. Expand the github workflows associated with the PDO repository. The old "ci" workflow has been moved to a workflow called "full_test" and is used as a condition for merge of PRs. It will no longer be run on every push but only on PR creation. Add a "configured_test" workflow that can be dispatched on demand. This workflow allows for different build and run parameters to be set interactively. Add a "build_docker" workflow that will attempt to build docker images pdo_client, pdo_services and pdo_ccf when a PR is merged successfully. This workflow can also be dispatched interactively. Signed-off-by: Mic Bowman <[email protected]>
|
|
||
| - name: Build and run tests | ||
| if: "!contains(github.event.commits[0].message, '[debug]')" |
There was a problem hiding this comment.
The old way of triggering debug builds clearly was hacky. But maybe would be good to add a comment to configured_test.yml and the possibility to manually trigger now parameterized builds?
There was a problem hiding this comment.
Good point. I will document it better.
There was a problem hiding this comment.
Small amount of documentation added.
| workflow_dispatch: | ||
|
|
||
| pull_request: | ||
| types: [closed] |
There was a problem hiding this comment.
Before seeing below if, i stumbled over this [closed] and the lack of something like a type [merged]. Googling a bit i found https://github.com/orgs/community/discussions/26724#discussioncomment-3253093 which proposes an arguably simpler solution to achieve the same effect: on: push: branches: - main
Oops, i notice the same discussion later also mentions your pattern. I didn't see though any problem with above pattern which still seems moderately simpler/more robust (e.g., you don't have to match other triggers in the if clause ..)
There was a problem hiding this comment.
Actually
- The comment at the top of that issue is actually wrong (you can detect a merged PR)
- Even in that series of responses, the conclusion is to use the pull_request event with a check for merged
I'm going to leave this as is.
| jobs: | ||
| pdo_specific_tests: | ||
| name: Run specific PDO tests | ||
| runs-on: ubuntu-22.04 |
There was a problem hiding this comment.
This could be another parameter (input)? Although admittedly as everything is done in docker, if we would parameterize that it probably would be better to parameterize docker's UBUNTU_VERSION build argument?
There was a problem hiding this comment.
good point. if i can actually use the variables from the dispatch i will add this. it will probably have to be fixed in a later PR though since i have no way to test it right now.
|
|
||
| if: > | ||
| github.event.name == 'workflow_dispatch' || | ||
| github.event.name == 'pull_request' && github.event.pull_request.merged == true |
There was a problem hiding this comment.
Right now we would generate the dockers for every PR, whether it is a version upgrade or not. But i guess there is no easy way to test whether this is a version upgrade PR? That said, as long as storage is not an issue, i guess there is loittle cost for doing it on all PRs (the actual build should be "free" if github caching works properly as the CI already should have built them?)
There was a problem hiding this comment.
agree. but remember that the version is updated with every commit. i think there is a question that we need to talk through about when to trigger the action. i don't think this is an unreasonable starting point. i would prefer to get this version working and then come back and address the trigger conditions.
Signed-off-by: Mic Bowman <[email protected]>
EXPERIMENTAL -- new github workflows will only be visible and runnable when they are attached to the main branch. this commit will enable further testing.
Expand the github workflows associated with the PDO repository. The old "ci" workflow has been moved to a workflow called "full_test" and is used as a condition for merge of PRs. It will no longer be run on every push but only on PR creation and synchronization (when updates are pushed).
Add a "configured_test" workflow that can be dispatched on demand. This workflow allows for different build and run parameters to be set interactively.
Add a "build_docker" workflow that will attempt to build docker images pdo_client, pdo_services and pdo_ccf when a PR is merged successfully. This workflow can also be dispatched interactively.