Skip to content

Conversation

@willcl-ark
Copy link
Contributor

Splits our single workflow into two. This means that we can use on: pull_request, which works much better™️ with other actions (like checkout).

It also means (I think) that PRs which themselves modify the workflow(s), should be exercised in the PR (unconfirmed, but I think it should work like that), which grant has been running into a few times recently...

The rationale for on: pull_request_target was that the github token given to on: pull_request does not have enough permissions to read repo secrets and things, so it could not deploy. This is not so with pull_request_target.

By splitting deploy into on: workflow, the token its natively given has the appropriate permissions.

The image from the build job is shared with the test job in the same workflow as an artifact. It is not shared directly (e.g. as an artifact) with the deploy workflow, but the build steps use a shared cache (see cache_from and cache_to), which achieves basically the same result.

@willcl-ark
Copy link
Contributor Author

I have two additional commits here which we can also look to add in the future if wanted

@mplsgrant
Copy link
Collaborator

I tried this out on my branch. It seemed to work, including against my new get_service_ip test.

@willcl-ark
Copy link
Contributor Author

Going to merge this now then, I think it's working too, and should be quite a big improvement to be actually testing the PR branch code!

@willcl-ark willcl-ark merged commit 725c1f1 into bitcoin-dev-project:main Jun 4, 2024
@pinheadmz
Copy link
Contributor

looks like the CI on this PR ran twice (pull_request and pull_request_target?) but dont see that on the merge to main

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants