Skip to content

Use official Ruby setup GH action#8614

Merged
jekyllbot merged 5 commits intojekyll:masterfrom
seshrs:ruby-action-fix
May 16, 2021
Merged

Use official Ruby setup GH action#8614
jekyllbot merged 5 commits intojekyll:masterfrom
seshrs:ruby-action-fix

Conversation

@seshrs
Copy link
Copy Markdown
Contributor

@seshrs seshrs commented Mar 24, 2021

This is a CI fix (housekeeping).

Summary

I noticed that the CI results in #8587 mentioned that actions/setup-ruby is no longer maintained and that we should migrate to using ruby/setup-ruby instead.

This PR updates jobs to use ruby/setup-ruby. I'm not super-familiar with Ruby and had to guess which steps were obsolete with the new action. Hopefully, the GH Actions results on this PR will help evaluate whether I got it right! 😅

EDIT: Haha that didn't work. Will try debugging later this week.

@seshrs
Copy link
Copy Markdown
Contributor Author

seshrs commented Mar 24, 2021

Oh I see some overlap with #8610. Will wait for that to merge first.

@MichaelCurrin
Copy link
Copy Markdown
Contributor

Looks good but I see there are conflicts to fix and this is WIP.

Copy link
Copy Markdown
Contributor

@MichaelCurrin MichaelCurrin left a comment

Choose a reason for hiding this comment

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

Fix conflicting file.

@seshrs
Copy link
Copy Markdown
Contributor Author

seshrs commented Mar 29, 2021

@MichaelCurrin yes you were right, the bundle env variables had to be declared at the job-level so that other steps use the correct bundle config! Thanks for pointing that out 😃

Given that the Third-Party Repository Profiling workflow completed successfully, I think we have more confidence that the docs workflow will also work when this PR is merged with master.

@seshrs seshrs marked this pull request as ready for review March 29, 2021 19:55
@MichaelCurrin
Copy link
Copy Markdown
Contributor

MichaelCurrin commented Mar 30, 2021

Yes good that the checks are passing.

And the job is setup to run on a PR against master and will work the same once merged.

@MichaelCurrin
Copy link
Copy Markdown
Contributor

You can find your branch marked with passing runs on the Actions tab

https://github.com/jekyll/jekyll/actions

@seshrs
Copy link
Copy Markdown
Contributor Author

seshrs commented Apr 21, 2021

Hi @MichaelCurrin, sorry I dropped the ball for a couple of weeks 😅

Is there some further action needed from me to push this PR forward?

@MichaelCurrin
Copy link
Copy Markdown
Contributor

No PR changes are outstanding. This just needs a review and merging by a maintainer - a reviewer with write access

@DirtyF DirtyF requested review from a team, mattr- and parkr and removed request for a team May 14, 2021 19:16
Copy link
Copy Markdown
Member

@parkr parkr left a comment

Choose a reason for hiding this comment

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

Looks good to me!

I wasn't able to fully validate the bundler-cache option but I'm assuming we'll be able to continue benefitting from the Actions cache.

@MichaelCurrin
Copy link
Copy Markdown
Contributor

MichaelCurrin commented May 15, 2021

More info on bundler cache flag here:

https://github.com/ruby/setup-ruby#single-job

That means that in this PR, we use the ruby/setup-ruby action to handle Bundle itself, bundle install step and cache of gems too. And so the cache action and the gem install steps are unnecessary and are good to be taken out.

@DirtyF
Copy link
Copy Markdown
Member

DirtyF commented May 16, 2021

@jekyll: merge +dev

@jekyllbot jekyllbot merged commit 56ef270 into jekyll:master May 16, 2021
@jekyllbot jekyllbot added the fix label May 16, 2021
jekyllbot added a commit that referenced this pull request May 16, 2021
@seshrs seshrs deleted the ruby-action-fix branch May 18, 2021 21:31
@jekyll jekyll locked and limited conversation to collaborators May 18, 2022
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.

5 participants