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

Build docs site with GitHub Actions #8201

Merged
merged 8 commits into from Nov 11, 2020
Merged

Build docs site with GitHub Actions #8201

merged 8 commits into from Nov 11, 2020

Conversation

iBug
Copy link
Contributor

@iBug iBug commented May 22, 2020

This is essentially a rework of #8126.

Key points and notable features:

  • Use GITHUB_TOKEN provided by GitHub Actions for authentication

  • Cache installation from Bundler

    • The cache depends on both Gemfile and jekyll.gemspec from the repository root. As Jekyll is always in rapid development, I chose to always run bundle install, regardless of whether there's a cache hit.
    • Use a global environment variable to specify Ruby version (Reference)
  • Generate prettier commit messages. Example:

    Deploy docs from 0123456789abcdef0123456789abcdef
    
    iBug: Build docs site with GitHub Actions
    
  • Good Bash script and Git practices, e.g.

    • Well-delimited variables (${VAR} instead of $VAR)
    • Quoted strings (cmd "$var" instead of cmd $var)
    • Use git push <remote> +<refspec> instead of git push -f

Note: Please add a commit to the gh-pages branch before merging this PR. As explained in the commit message of e8eb528, GitHub Pages currently has a bug where it won't deploy properly on branches containing only one single commit. I am working around this issue by adding commits on top of the existing branch in many of my projects, and they're all working smoothly, so I'm carrying the same technique to this PR.


On a side note, previous failures from #8126 are (most likely) caused by an ongoing incident of GitHub. Specifically:

We have identified the source causing elevated errors as well as occasional stale data on GitHub.com. We are working on remediation.

As old GITHUB_TOKENs are supplied to Actions runs (they expire 60 minutes after creation), git push failed to authenticate. This should not happen normally so it'll be settled automatically after GitHub recovers.


Please feel free to share any concerns you have with my implementation. I'll be eager to answer them.

@iBug

This comment has been minimized.

@MichaelCurrin
Copy link
Contributor

MichaelCurrin commented Jul 11, 2020

This workflow is heavy. It would be abstracted in a simple workflow like this one:

https://github.com/BryanSchuetz/jekyll-deploy-gh-pages#example

As recommended by DirtyF in another thread github/pages-gem#651 (comment)

Or see this approach which is what the Jekyll docs recommend.

https://jekyllrb.com/docs/continuous-integration/github-actions/#setting-up-the-action

I added the article by the way and my demo listed at the end works now using GITHUB_TOKEN not the JEKYLL_PAT as in the tutorial. UPDATE: This failed - use JEKYLL_PAT

@MichaelCurrin
Copy link
Contributor

Oh I see in the original PR

(Use an in-house module instead of third-party action for greater control over the process)

What specific control is needed that can't be done with the third party ones and options or additional build steps?

@ashmaroli
Copy link
Member

What specific control is needed that can't be done with the third party ones

Third-party services are always tailored for the generic userbase.
That is why there are always provisions to customize a procedure.

Having a completely in-house procedure for something as simple as setting up a shell script and a YAML config file will allow
control over the overall log output and commit messages.

@MichaelCurrin
Copy link
Contributor

What specific control is needed that can't be done with the third party ones

Third-party services are always tailored for the generic userbase.
That is why there are always provisions to customize a procedure.

Having a completely in-house procedure for something as simple as setting up a shell script and a YAML config file will allow
control over the overall log output and commit messages.

Thanks I am onboard with this approach then. There is more control, but at the cost of having to maintain boilerplate instead of having it maintained in a separate action (which means less control and a different kind of risk of commands going out of date).

As of July 31, 2020, GitHub Pages still fails to deploy if the target
branch contains only one commit (the root commit). By cloning the target
branch before building, we can push the newly built content as a new
commit on top of the existing branch, effectively bypassing this GH
Pages issue.

Also, this preserves the deploy history, though I'm unsure if anyone
really cares.
@iBug
Copy link
Contributor Author

iBug commented Aug 5, 2020

I've got some new updates, namely issues with GitHub Pages deployment. Please kindly check the commit message of e8eb528 and the PR description, which I've updated to explain what I'm doing.

@MichaelCurrin
Copy link
Contributor

@iBug Thanks for updating the token as suggested. The other commits and message make sense.

@DirtyF DirtyF requested a review from a team November 11, 2020 12:52
Copy link
Member

@mattr- mattr- left a comment

Choose a reason for hiding this comment

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

I'd like to see the gem invokations go away before this gets merged.

.github/workflows/docs.yml Outdated Show resolved Hide resolved
.github/workflows/docs.yml Outdated Show resolved Hide resolved
@DirtyF DirtyF requested a review from mattr- November 11, 2020 16:58
Copy link
Member

@mattr- mattr- left a comment

Choose a reason for hiding this comment

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

LGTM.

Very cool to see us use this for site building. 🎉

@DirtyF
Copy link
Member

DirtyF commented Nov 11, 2020

@jekyll: merge +docs

@jekyllbot jekyllbot merged commit 10943d1 into jekyll:master Nov 11, 2020
jekyllbot added a commit that referenced this pull request Nov 11, 2020
@iBug

This comment has been minimized.

github-actions bot pushed a commit that referenced this pull request Nov 11, 2020
iBug: Build docs site with GitHub Actions (#8201)

Merge pull request 8201
github-actions bot pushed a commit to patilswapnilv/jekyll that referenced this pull request Nov 11, 2020
jekyllbot: Update history to reflect merge of jekyll#8201 [ci skip]
@DirtyF

This comment has been minimized.

@iBug

This comment has been minimized.

@iBug iBug deleted the build-docs branch November 11, 2020 20:28
@DirtyF
Copy link
Member

DirtyF commented Nov 11, 2020

https://jekyllrb.com is now built with current master and deployed in 25s total. 🎉

@jekyll jekyll locked and limited conversation to collaborators Nov 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants