Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run deploy only after successull check #89

Merged
merged 2 commits into from
Mar 3, 2023
Merged

Run deploy only after successull check #89

merged 2 commits into from
Mar 3, 2023

Conversation

kalambet
Copy link
Member

Description

This PR updates GH Actions workflow so it runs CfloudFlare Page deploy only after CI check compelted successfully.

@mpetrunic
Copy link
Member

mpetrunic commented Feb 21, 2023

Will this work? isnt cloudflare deploy relying on some push event details like ref?

@kalambet
Copy link
Member Author

kalambet commented Feb 21, 2023

@mpetrunic Ideally it would be nice to disable CloudFlare autodeploy on the ClpudFlare side completly (you can check how Gaming Docs are working). Bc right now we are sorta have two deploys. One initiated by watchers of the repo on CloudFlare side and another one initiated by GitHub Actions.

If we are proactively doing deploy from here you do not need git ref or anything. The buld is actually happening inside GH Action.

The downside - there is no nice comment with deployerd URL inside PR itslef. The plus side - you still can see this data in the comment on GitHub workflow page like this:
https://github.com/ChainSafe/game-docs-v2/actions/runs/4234551979

@kalambet kalambet self-assigned this Feb 21, 2023
@mpetrunic
Copy link
Member

@mpetrunic Ideally it would be nice to disable CloudFlare autodeploy on the ClpudFlare side completly (you can check how Gaming Docs are working). Bc right now we are sora have two deploys. One initiated by watchers of the repo on CloudFlare side and another one initiated by GitHub Actions.

If we are proactively doing deploy from here you do not need git ref or anything. The buld is actually happening inside GH Action.

The downside - there is no nice comment with deployerd URL inside PR itslef. The plus side - you still can see this data in the comment on GitHub workflow page like this: https://github.com/ChainSafe/game-docs-v2/actions/runs/4234551979

Another downside is that Cloudflare keeps the URL the same if you push from the same ref. We could figure out a way to hack but maybe it's easier to add to the ci script as the last task?

@mpetrunic
Copy link
Member

But also cloudflare deploy didn't even run here or created a deployment here

@kalambet
Copy link
Member Author

kalambet commented Feb 21, 2023

Yep, that is becasue workflow_run is not triggering until it's on the default branch. https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#workflow_run

There second Note about it. But after the merge it works as expected. The Gaming Docs repo working in this exact way.

@kalambet
Copy link
Member Author

The only thing that I generally wanted to achieve here is to make sure build is happening on the GH Actions side and we will not have the same problem as with Render when sometimes their watchers was broken or sometimes did not even report the problem back.



But I'm totally open for any other solutions to this.

Copy link
Member

@mpetrunic mpetrunic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let see how it goes and we can revert if it doesn't work. But I feel like we should put this in docs

@mpetrunic mpetrunic merged commit cb7a385 into main Mar 3, 2023
@mpetrunic mpetrunic deleted the peter/gh-action branch March 3, 2023 11:50
@kalambet
Copy link
Member Author

kalambet commented Mar 3, 2023

Oh, yes, agree. Let me add docs also to this PR so it is more or less a single package. And if we decide to roll it back we will remove the related docs too.

@kalambet
Copy link
Member Author

kalambet commented Mar 3, 2023

Ah, I just realised that this one is closed. Will create another one.

@mpetrunic
Copy link
Member

Ah, I just realised that this one is closed. Will create another one.

I already opened one #92 But feel free to add reasoning regarding workflow trigger there!

@mpetrunic mpetrunic mentioned this pull request Mar 3, 2023
@mpetrunic
Copy link
Member

@kalambet Maybe take a look at #93

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.

None yet

2 participants