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.

Sign up for GitHub

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

Potential follow up fix for #125 #129

Merged
merged 5 commits into from
Oct 25, 2024
Merged

Potential follow up fix for #125 #129

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
793d75d
Potential follow up fix for #125
jbusecke Oct 17, 2024
5d315ae
Add checkout to deploy workflow
jbusecke Oct 18, 2024
7d02834
Update deploy-book.yaml
jbusecke Oct 18, 2024
e0348b9
Update deploy-book.yaml
jbusecke Oct 18, 2024
1393c0f
Update deploy-book.yaml
jbusecke Oct 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 21 additions & 13 deletions .github/workflows/deploy-book.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ jobs:
run:
shell: bash -l {0}
steps:
- uses: actions/checkout@v4

- name: Download merged artifact
if: inputs.is_preview != 'true'
uses: actions/download-artifact@v4
Expand All @@ -59,27 +61,33 @@ jobs:
unzip book.zip
rm -f book.zip

- name: Debug
run: pwd

- name: Debug
run: ls -la book/_build/html

- name: Deploy to GitHub Pages
uses: JamesIves/github-pages-deploy-action@v4
if: |
(github.ref == 'refs/heads/main' && inputs.cname == 'None')
with:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
BRANCH: gh-pages
FOLDER: ${{ inputs.publish_dir }}
CLEAN: true
KEEP_HISTORY: false
BASE_PATH: ${{ inputs.destination_dir }}
token: ${{ secrets.GITHUB_TOKEN }}
branch: gh-pages
folder: ${{ inputs.publish_dir }}
clean: true
clean-exclude: "preview" # keep existing previews from other PRs
target-folder: ${{ inputs.destination_dir }}

- name: Deploy to GitHub Pages with custom domain
uses: JamesIves/github-pages-deploy-action@v4
if: |
(github.ref == 'refs/heads/main' && inputs.cname != 'None')
with:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
BRANCH: gh-pages
FOLDER: ${{ inputs.publish_dir }}
CLEAN: true
KEEP_HISTORY: false
BASE_PATH: ${{ inputs.destination_dir }}
CNAME: ${{ inputs.cname }}
token: ${{ secrets.GITHUB_TOKEN }}
branch: gh-pages
folder: ${{ inputs.publish_dir }}
clean: true
clean-exclude: "preview" # keep existing previews from other PRs
target-folder: ${{ inputs.destination_dir }}
CNAME: ${{ inputs.cname }} # how can we set this for the new action?
Copy link
Contributor Author

@jbusecke jbusecke Oct 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

CNAME does not have an analog in the new action. Does someone have an example deployment that uses this input to test. There is some more info here: https://github.com/JamesIves/github-pages-deploy-action#additional-build-files-

Copy link
Contributor

Choose a reason for hiding this comment

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

Could git-config-name be similar to what you are looking for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am honestly really unsure. Could somebody test this out on a doc that uses this feature?

Copy link
Member

Choose a reason for hiding this comment

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

Our Foundations site uses this feature. Currently fixing some broken links in Foundations that are preventing CI tests from passing (ProjectPythia/pythia-foundations#495), once those are merged I'll do some testing here.

Copy link
Member

Choose a reason for hiding this comment

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

I did a bit of reading on the CNAME feature, including the docs that @jbusecke linked above. Previously we were passing this argument through to peaceiris/actions-gh-pages. As far as I understand, the only effect of this argument is to ensure that a CNAME file is created at the root of the deployment branch (we use gh-pages, e.g. this file in the Foundations repo).

That file hasn't changed in three years because we've never changed the custom domain name.

The docs for peaceiris/actions-gh-pages also say

To add the CNAME file, we can set the cname option. Alternatively, put your CNAME file into your publish_dir. (e.g. public/CNAME)

The docs for JamesIves/github-pages-deploy-action also refer to manual commit of the CNAME file into the deployment branch:

If you're using a custom domain and require a CNAME file, or if you require the use of a .nojekyll file, you can safely commit these files directly into the deployment branch without them being overridden after each deployment

My conclusion is that we should just remove this input argument. Manually creating the CNAME file in the gh-pages branch is a one-time step. And within Project Pythia, we only use this feature for the Foundations and Cookbook Gallery repos. It's not something that we are actively supporting for Cookbook contributors.