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

Remove webrick dependency #118

Merged
merged 3 commits into from
Aug 30, 2023
Merged

Remove webrick dependency #118

merged 3 commits into from
Aug 30, 2023

Conversation

machikoyasuda
Copy link
Member

@machikoyasuda machikoyasuda commented Aug 24, 2023

Related to #92 #93

Testing out to see if the app runs fine locally and on build without the explicit webrick dependency.

github/pages-gem#752 - but we use Ruby 2.7, so we shouldn't be affected by this.

Would need to get everyone to test this on their local machines, to make sure the app still runs locally for them.

Strangely, the Mobi Mart repo https://github.com/cal-itp/mobility-marketplace/pulls is having different results with the same dependabot upgrades:
image

Calitp.org doesn't have Webrick installed, and it was able to upgrade Jekyll just fine: https://github.com/cal-itp/calitp.org/blob/main/Gemfile

@machikoyasuda machikoyasuda requested a review from a team as a code owner August 24, 2023 23:52
@machikoyasuda machikoyasuda self-assigned this Aug 24, 2023
@netlify
Copy link

netlify bot commented Aug 24, 2023

Deploy Preview for compilerla ready!

Name Link
🔨 Latest commit d415d83
🔍 Latest deploy log https://app.netlify.com/sites/compilerla/deploys/64efb36913202a0008fa64bf
😎 Deploy Preview https://deploy-preview-118--compilerla.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

terminal-table (3.0.2)
unicode-display_width (>= 1.1.1, < 3)
unicode-display_width (2.4.2)
webrick (1.7.0)

PLATFORMS
aarch64-linux
Copy link
Member Author

Choose a reason for hiding this comment

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

My machine does this!!!


BUNDLED WITH
2.3.22
2.4.13
Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know why or how, but my devcontainer has a higher version of bundler.

Copy link
Member

@thekaveman thekaveman left a comment

Choose a reason for hiding this comment

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

So I checked out this branch, totally rebuilt the devcontainer, and when I went to start the Jekyll site, I get this error:

Executing task: jekyll serve --force_polling --livereload 

/var/lib/gems/3.1.0/gems/bundler-2.4.13/lib/bundler/runtime.rb:304:in `check_for_activated_spec!': 
You have already activated webrick 1.8.1, but your Gemfile requires webrick 1.7.0. Prepending `bundle exec` to your command may solve this. (Gem::LoadError)
        from /var/lib/gems/3.1.0/gems/bundler-2.4.13/lib/bundler/runtime.rb:25:in `block in setup'
        from /var/lib/gems/3.1.0/gems/bundler-2.4.13/lib/bundler/spec_set.rb:165:in `each'
        from /var/lib/gems/3.1.0/gems/bundler-2.4.13/lib/bundler/spec_set.rb:165:in `each'
        from /var/lib/gems/3.1.0/gems/bundler-2.4.13/lib/bundler/runtime.rb:24:in `map'
        from /var/lib/gems/3.1.0/gems/bundler-2.4.13/lib/bundler/runtime.rb:24:in `setup'
        from /var/lib/gems/3.1.0/gems/bundler-2.4.13/lib/bundler.rb:162:in `setup'
        from /var/lib/gems/3.1.0/gems/jekyll-4.3.2/lib/jekyll/plugin_manager.rb:52:in `require_from_bundler'
        from /var/lib/gems/3.1.0/gems/jekyll-4.3.2/exe/jekyll:11:in `<top (required)>'
        from /usr/local/bin/jekyll:25:in `load'
        from /usr/local/bin/jekyll:25:in `<main>'

 *  The terminal process "/bin/bash '-c', 'jekyll serve --force_polling --livereload'" terminated with exit code: 1. 
 *  Terminal will be reused by tasks, press any key to close it.

So I'm not sure why this isn't working here where it does work in calitp.org

@machikoyasuda
Copy link
Member Author

machikoyasuda commented Aug 30, 2023

Ah, it's because both MobiMart and calitp.org have bundle exec before the jekyll command, but this one doesn't:

https://github.com/cal-itp/mobility-marketplace/blob/main/.vscode/tasks.json
https://github.com/cal-itp/calitp.org/blob/main/.vscode/tasks.json
image

Prepending bundle exec makes sure the app uses the versions from the Gemfile: https://bundler.io/v2.4/man/bundle-exec.1.html Not using bundle exec before the command means that Ruby might be using whatever version it finds, which might not matchw ith what's on the Gemfile.

Copy link
Member

@thekaveman thekaveman left a comment

Choose a reason for hiding this comment

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

It works! 🎉

Copy link
Member

@angela-tran angela-tran left a comment

Choose a reason for hiding this comment

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

This worked for me! Thanks for figuring this out @machikoyasuda

@machikoyasuda
Copy link
Member Author

@angela-tran @thekaveman Thanks for confirming!

@machikoyasuda machikoyasuda merged commit 565d974 into main Aug 30, 2023
4 checks passed
@machikoyasuda machikoyasuda deleted the chore/gemfile branch August 30, 2023 22:31
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