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

[#182] Local github page deployment fails on Ruby >=3.0 #185

Conversation

Eclipse-Dominator
Copy link
Contributor

Fixes #182

Running bundle exec jekyll serve to test GitHub page deployment will fail due to a compatibility issue with Jekyll 3 and Ruby >=3.0.

Let's resolve this by

  • adding "webrick" to Gemfile
  • updating the relevant packages

@canihasreview
Copy link

canihasreview bot commented Jun 2, 2023

Click here to submit a new iteration when this PR is ready for review.

See this repository's contribution guide for more information.

@Eclipse-Dominator Eclipse-Dominator force-pushed the 182-local-github-page-deployment-fails-on-ruby branch from e275d14 to 20e1834 Compare June 2, 2023 13:37
@Eclipse-Dominator Eclipse-Dominator changed the title [#182] Fix local GitHub page deployment failure [#182] Local github page deployment fails on Ruby >=3.0 Jun 2, 2023
@canihasreview
Copy link

canihasreview bot commented Jun 2, 2023

v1

@Eclipse-Dominator submitted v1 for review.

(📚 Archive)

Checkout this PR version locally
git fetch https://github.com/se-edu/addressbook-level3.git refs/pr/185/1/head:BRANCHNAME

where BRANCHNAME is the name of the local branch you wish to fetch this PR to.

@codecov
Copy link

codecov bot commented Jun 2, 2023

Codecov Report

Merging #185 (b2d03aa) into master (481a3d9) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #185   +/-   ##
=========================================
  Coverage     72.06%   72.06%           
  Complexity      399      399           
=========================================
  Files            70       70           
  Lines          1235     1235           
  Branches        127      127           
=========================================
  Hits            890      890           
  Misses          314      314           
  Partials         31       31           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@damithc damithc left a comment

Choose a reason for hiding this comment

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

Looks OK to me, but I am not very familiar with Ruby or Jekyll. Let's leave it here for a few days in case others have any comments.

Commit message:

  • Try to make the commit subject more specific so that it tell us what exact problem was fixed.
  • The local testing is nothing to do with GitHub Pages. It's just a Jekyll server.

Running `bundle exec jekyll serve` to start a Jekyll server
to serve the application's documentation page will crash
due to a compatibility issue with Jekyll 3 and Ruby >=3.0.

Let's resolve this by
- adding "webrick" to Gemfile
- updating the relevant packages
@Eclipse-Dominator Eclipse-Dominator force-pushed the 182-local-github-page-deployment-fails-on-ruby branch from 20e1834 to b2d03aa Compare June 2, 2023 21:35
@canihasreview
Copy link

canihasreview bot commented Jun 2, 2023

v2

@Eclipse-Dominator submitted v2 for review.

(📚 Archive) (📈 Interdiff between v1 and v2) (📈 Range-Diff between v1 and v2)

Checkout this PR version locally
git fetch https://github.com/se-edu/addressbook-level3.git refs/pr/185/2/head:BRANCHNAME

where BRANCHNAME is the name of the local branch you wish to fetch this PR to.

@damithc
Copy link
Contributor

damithc commented Jun 6, 2023

No further comments on the fix but a couple of questions, mostly out of curiosity.

  • Why does adding webricks fix the problem?
  • How did you decide those packages need to be upgraded to those exact versions?

@Eclipse-Dominator
Copy link
Contributor Author

I think prior to ruby 3.0, "webricks" package is installed by default. That's why after ruby 3.0, jekyll server will fail since they can no longer find the webricks package.

I think those were the version numbers after I updated ruby packages. I think I can fine tune the versions more? Alternatively, just removing gem.lock file and let the users install the latest version will work as well

@damithc
Copy link
Contributor

damithc commented Jun 8, 2023

I have no further comments on this PR.
I also don't know enough about Jekyll to decide whether we should keep gem.lock in the repo or remove it.
@se-edu/tech-team-level1 feel free to weigh in.

Copy link

@dlimyy dlimyy left a comment

Choose a reason for hiding this comment

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

I tested it on my end and LGTM! I think it is better to keep the gem.lock file just in case the newer versions of the gems installed by bundler causes the website to crash.

@damithc damithc merged commit f0e9069 into se-edu:master Jun 12, 2023
@damithc
Copy link
Contributor

damithc commented Jun 12, 2023

Thanks for the fix @Eclipse-Dominator
Note that I edited the commit message slightly, to explain the reason for the crash so that the reason for the change is evident.

@Eclipse-Dominator Eclipse-Dominator deleted the 182-local-github-page-deployment-fails-on-ruby branch June 12, 2023 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Local github page deployment fails on Ruby >=3.0
3 participants