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

Update minimum Ruby version and gems #424

Merged
merged 1 commit into from
May 2, 2024
Merged

Conversation

ctubbsii
Copy link
Member

@ctubbsii ctubbsii commented May 1, 2024

  • Update minimum Ruby version to at least the version run by ASF infra team on Jekyll builders, so that way we can reduce compatibility surprises when trying to build on stuff substantially newer than what's on ASF's infrastructure
  • Do a bundle update

* Update minimum Ruby version to at least the version run by ASF infra
  team on Jekyll builders, so that way we can reduce compatibility
  surprises when trying to build on stuff substantially newer than
  what's on ASF's infrastructure
* Do a bundle update
@ctubbsii ctubbsii requested review from EdColeman and ddanielr May 1, 2024 22:24
@ctubbsii ctubbsii self-assigned this May 1, 2024
@ctubbsii
Copy link
Member Author

ctubbsii commented May 1, 2024

@clambertus confirmed in Slack that the builder is running Ruby 2.7 (I think it's safe to rely on it not going backwards). I don't think any of these gems require anything newer than 2.7, but I won't know for sure until we commit to trigger the site build with the changes.

Maybe we can add a matrix build for GitHub Actions that tests 2.7 and also 3.3, and we can update the Dockerfile to use 3.3? I'm not sure what's useful to test here. At a certain point, it's probably overkill.

What I do know is these updated gems work fine on Ruby 3.3.0 on my Fedora 40 dev machine. I imagine they work fine with the 3.2.2 version used by our Docker container for testing locally, but I can't test that, since I don't have Docker set up locally. Maybe @ddanielr or @DomGarguilo has that set up already and can test it?

Copy link
Member

@DomGarguilo DomGarguilo left a comment

Choose a reason for hiding this comment

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

LGTM - Works for me when using the development docker container

Copy link
Contributor

@EdColeman EdColeman left a comment

Choose a reason for hiding this comment

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

The changes look okay - I'll defer to @DomGarguilo for running it in docker.

@ctubbsii ctubbsii merged commit d6679b2 into apache:main May 2, 2024
1 check passed
@ctubbsii ctubbsii deleted the bundle-update branch May 2, 2024 23:41
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.

3 participants