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

Add PR preview builds to GitHub Pages #68

Merged
merged 1 commit into from
May 17, 2023

Conversation

oraNod
Copy link
Contributor

@oraNod oraNod commented May 3, 2023

Fixes #16

This PR:

  • Uses the JamesIves action to deploy to gh-pages instead of the Nikola action. This allows us to avoid overwriting the pr-previews branch.
  • Uses the deploy-pr-preview action to create PR preview builds of the community site.

@oraNod
Copy link
Contributor Author

oraNod commented May 3, 2023

To test the changes in this PR I did the following:

Opened a PR against my fork that created a preview build: oraNod#8

Checked that the preview was available as expected at: https://oranod.github.io/community-website/previews/pr-8/

Opened a second PR against my fork and created another preview build.

Merged the second PR to ensure that the first preview was not overwritten, the second PR preview was deleted, and the main site was updated.

To verify you can view the history for my gh-pages branch: https://github.com/oraNod/community-website/commits/gh-pages

PR 8 is the first test commit. PR 9 is the second preview that was merged to main.

@GregSutcliffe
Copy link
Contributor

GregSutcliffe commented May 9, 2023

@oraNod Interestingly this PR seems to be using the new action, and it's throwing some errors in the checks. I see chmod failures, and this as well:

Run echo "::set-output name=url::https://ansible-community.github.io/community-website//previews//pr-68/"
Warning: The `set-output` command is deprecated and will be disabled soon. Please upgrade to using Environment Files.

Do we need to reconfigure the repo itself to use Environments when we merge this?

@oraNod
Copy link
Contributor Author

oraNod commented May 9, 2023

@GregSutcliffe Yeah I think this is why you have to be really really careful about doing things like putting secrets in workflows for repos that accept public PRs.

I also saw those warns about the deprecated commands earlier today too and started looking. I think they come from the PR preview action here: https://github.com/rossjrw/pr-preview-action/blob/506bcd891ec5c215ce8857e128d50b9d698eb8cc/action.yml#L137

And it looks like there's already a PR to fix it: rossjrw/pr-preview-action#35

@oraNod
Copy link
Contributor Author

oraNod commented May 9, 2023

@GregSutcliffe I just commented in the PR that will fix the deprecated command warning for that action. Maybe we should hold off on merging this until that is fixed. I'll convert this PR to a draft.

@oraNod oraNod marked this pull request as draft May 9, 2023 16:18
@GregSutcliffe
Copy link
Contributor

@GregSutcliffe I just commented in the PR that will fix the deprecated command warning for that action. Maybe we should hold off on merging this until that is fixed. I'll convert this PR to a draft.

I'd agree with waiting until it's fixed - do you have the link to that issue?

@oraNod
Copy link
Contributor Author

oraNod commented May 16, 2023

@oraNod oraNod marked this pull request as ready for review May 16, 2023 13:55
@oraNod
Copy link
Contributor Author

oraNod commented May 16, 2023

@GregSutcliffe That action was updated to remove the deprecated option and the warning is no longer present: https://github.com/ansible-community/community-website/actions/runs/4992789876/jobs/8940972958#step:6:651

It's still failing with a permissions thing, which is related to the GH actions token. It looks like permissions are OK in the repo settings and I don't know if we need to specify permissions in the action itself. Maybe something like:

permissions:
  actions: write

Perhaps it is just the case that the action is trying to run from an unmerged PR and is failing for that reason. I have tested this on my fork and am pretty confident it'll work as expected after merge.

@GregSutcliffe
Copy link
Contributor

Fair enough. Let's merge it and see, we can fix it up as we go.

@GregSutcliffe GregSutcliffe merged commit 8273f00 into ansible-community:main May 17, 2023
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.

Preview builds
2 participants