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

Jump to bottom

GitHub actions: deployment to GitHub Pages, link check #198

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

GitHub actions: deployment to GitHub Pages, link check #198

wants to merge 3 commits into from

Conversation

deining
Copy link
Collaborator

@deining deining commented Feb 23, 2023 •
edited
Loading

This PR follows up and closes #161.
It adds automated workflows for

  • site deployment on GitHub pages
  • link checking

For this purpose, two workflow files for GitHub actions were added.
Workflow actions can be either run manually (via GitHub web UI) or on each push and/or pull request.
Two sections were added to README.md, describing the newly available features and their configuration.

Note:: In order to allow the workflow to cache npm dependencies, packages.json.lock was added to the repo.

@deining deining requested review from chalin and LisaFC February 23, 2023 10:03
@deining deining mentioned this pull request Feb 23, 2023
Merged
@deining deining force-pushed the deploy-to-github-pages branch from 0e66e5a to 25aeb03 Compare February 24, 2023 16:29
chalin
chalin reviewed Mar 1, 2023
View reviewed changes
Copy link
Collaborator

@chalin chalin left a comment

Choose a reason for hiding this comment

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

The example is officially hosted by Netlify (which reminds me, this needs to be indicated on the homepage). A production build should not be hosted by two servers -- that would be bad for SEO and potentially confusing to readers.

/cc @geriom @LisaFC

@deining
Copy link
Collaborator Author

deining commented Mar 1, 2023 •
edited
Loading

A production build should not be hosted by two servers

I agree, and it was never my intention to deploy the example site twice.

My motivation for this PR is different: in the user guide we recommend using this repo as template when building a new site. We cannot assume, that all users host their site at netlify, some may opt for hosting their site at GitHub Pages. The GitHub Pages deployment workflow was added in order to make it as easy as possible for latter users to get their site deployed. We easily can (and should!) disable this workflow once my PR was merged. To sum up, this workflow has sample/demonstration character only and was never meant to be used for the actual deployment of the example site.

WDYT?

@deining deining requested a review from chalin March 1, 2023 19:17
@froboy
Copy link

froboy commented Jun 9, 2023

What if we moved these files from .github/workflows to something like .github/example-workflows so that they're in the repo, but not enabled out of the box. Then we could have some documentation instructing users to mv them to the proper directory if necessary, but they wouldn't do anything if they were not necessary.

@wangfenjin wangfenjin mentioned this pull request Jul 28, 2023
Open
@deining deining force-pushed the deploy-to-github-pages branch 2 times, most recently from 5a57839 to 8ad0273 Compare July 31, 2023 09:00
@deining
GitHub actions: deployment to GitHub Pages, link check
9ca7bbc 
* added workflow files
* README.md:
  - added section on deployment via GitHub pages
  - added section on automated link checking
@deining deining force-pushed the deploy-to-github-pages branch from 8ad0273 to 9ca7bbc Compare October 31, 2024 10:37
@deining
Copy link
Collaborator Author

deining commented Oct 31, 2024 •
edited
Loading

What if we moved these files from .github/workflows to something like .github/example-workflows

Great idea. I just did that by moving the files to .github/wokflows/example. I adapted the documentation accordingly, rebased my PR and brought all versions and dependencies in the workflow file up-to-date.
@chalin: Could you please take another look?

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

@LisaFC LisaFC Awaiting requested review from LisaFC

@chalin chalin Awaiting requested review from chalin

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Consider adding hyperlink dead-link checking to example site
3 participants
@deining @froboy @chalin