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

Recommend using bundler to ensure the correct dependencies are installed #257

Closed
wants to merge 3 commits into from
Closed

Recommend using bundler to ensure the correct dependencies are installed #257

wants to merge 3 commits into from

Conversation

matt-allan
Copy link

Hi Hoodies,

I was looking for a starter issue and saw the comments on this post about having trouble setting up jekyll locally. Then I tried and had the same issues :)

I was able to get everything working correctly by doing a bundle install and then using bundle exec jekyll * instead of jekyll *.

Since the jekyll-redirect-from and jekyll-paginate plugins are already specified in the github-pages gem, bundle install will always install the correct versions.

I wasn't sure why bundle exec worked better, but here is what I found on the bundler website:

In some cases, running executables without bundle exec may work, if the executable happens to be installed in your system and does not pull in any gems that conflict with your bundle.

However, this is unreliable and is the source of considerable pain. Even if it looks like it works, it may not work in the future or on another machine.

@gr2m
Copy link
Member

gr2m commented Mar 16, 2016

Thanks @yuloh, I can confirm that bundle exec also worked for me when I run into this.
LGTM 👍

@@ -92,12 +91,12 @@ Please fork the website and send a Pull Request for your contribution. Here is <

We will review the Pull Request (PR) and merge or comment it.

**Content changes and typos**
**Content changes and typos**
Copy link
Member

Choose a reason for hiding this comment

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

can you leave the white space? It’s needed for line breaks. Same in line 99

@matt-allan
Copy link
Author

Sorry, editor preferences 😄

@gr2m
Copy link
Member

gr2m commented Mar 23, 2016

I’ve another question, it’s been a while since my ruby days :) We have Gemfile.lock in .gitignore. The bundler website suggests to add it to git though: git add Gemfile Gemfile.lock. What do you think would be recommended for us?

@janl
Copy link
Member

janl commented Mar 23, 2016

If we do this, we need to add how to install bundler to the instructions ;)

> bundle install
-bash: bundle: command not found

Also, we are using https://github.com/jekyll/jekyll-redirect-from and it’s not defined in our Gemfile, we should all fix this in one go.

@janl
Copy link
Member

janl commented Mar 23, 2016

@yuloh great initiative! :)

@steveklabnik
Copy link

Yes, this is correct. bundle exec will ensure that only the gems in your Gemfile and their dependencies are in the path.

I’ve another question, it’s been a while since my ruby days :) We have Gemfile.lock in .gitignore. The bundler website suggests to add it to git though: git add Gemfile Gemfile.lock. What do you think would be recommended for us?

It should be in ignore for libraries, but not for applications. This site is an application, so it should be in source control, not ignored.

@matt-allan
Copy link
Author

Another option is to not commit the lock file and parse the versions.json provided by github, which would ensure the local version of the github-pages gem is always up to date. It's explained in the jekyll docs under the "Use the github-pages gem" heading. If we commit the gemfile.lock we will just need to run bundle update regularly.

If we do this, we need to add how to install bundler to the instructions ;)

Good point. I will push an update to the readme.

Also, we are using https://github.com/jekyll/jekyll-redirect-from and it’s not defined in our Gemfile, we should all fix this in one go.

I thought the same thing initially but it's actually installed by the github pages gem. All of the github pages gem dependencies are listed here. Since github pages builds with the --safe option, any dependencies not listed in the github-pages gem are ignored.

@janl
Copy link
Member

janl commented Mar 23, 2016

I thought the same thing initially but it's actually installed by the github pages gem.

@yuloh perfect!


This doesn't serve the actual jekyll page, though. To do that, run `jekyll serve --watch --drafts` as well, which will run the full page at `localhost:4000`, using the styles from the grunt task.
You may need to `gem install jekyll-redirect-from jekyll-paginate` beforehand.
The Hoodie website is built with [Jekyll](https://jekyllrb.com/), Jekyll requires ruby version 2.0.0 or higher. Open a terminal and type `ruby --version` to check if ruby is installed and up to date. If not, follow the [official download instructions](https://www.ruby-lang.org/en/downloads/). Now install [bundler](http://bundler.io/) by running `gem install bundler` on the command line.
Copy link
Author

Choose a reason for hiding this comment

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

These instructions are paraphrased from the github pages instructions.

@matt-allan
Copy link
Author

I got some clarification on the downsides of using the versions.json approach over here. Every time you do bundle exec jekyll serve it will make an HTTP call, which isn't really ideal.

I'm going to update this pull request to version the Gemfile.lock, since that seems like the best way to go. It does mean contributors will occasionally need to run bundle update github-pages to keep the local version in sync.

@@ -1,2 +1,2 @@
source 'https://rubygems.org'
gem 'github-pages'
gem 'github-pages', group: :jekyll_plugins
Copy link
Author

Choose a reason for hiding this comment

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

This change allows the github-pages gem to set jekyll config settings, like enabling Github flavored markdown. The full config it's using is in this file.

@gr2m
Copy link
Member

gr2m commented Mar 25, 2016

Tested it locally and worked flawlessly, great work everyone :) LGTM

@gr2m
Copy link
Member

gr2m commented Mar 25, 2016

@janl can you double check before we merge?

@gr2m
Copy link
Member

gr2m commented Apr 25, 2016

merged via 7b040c2, 44575e0, 412af70, a5d6910

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.

4 participants