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

Fixed error when the site.github config key is set, but site.github.url is not #43

Merged
merged 1 commit into from
Sep 8, 2014

Conversation

akoeplinger
Copy link
Contributor

Fixes #42

@@ -59,7 +59,8 @@ def redirect_prefix(site)
end

def config_github_url(site)
site.config.fetch('github', Hash.new).fetch('url', nil)
config_github = site.config.fetch('github', Hash.new)
config_github.respond_to?('fetch') ? config_github.fetch('url', nil) : nil
Copy link
Contributor

Choose a reason for hiding this comment

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

Just out of curiosity, do you think it would be cleaner to write this with an inline if statement?

config_github.fetch('url', nil) if config_github.respond_to?('fetch')

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, duh, don't mind me—you need the nil return value here. 😊

Sorry, I think my suggestion actually would return nil if config_github.respond_to?('fetch') returned false.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, your suggestion works too. I don't have a preference, but I don't do much Ruby, so not sure which one is considered more idiomatic 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe @parkr would know...

Copy link
Member

Choose a reason for hiding this comment

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

I'd probably write:

def config_github_url(site)
  github_config = site.config['github']
  github_config['url'] if github_config.is_a?(Hash)
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@parkr done

@kansaichris
Copy link
Contributor

Wow, that was fast. Thanks for opening this, @akoeplinger!

parkr added a commit that referenced this pull request Sep 8, 2014
@parkr parkr merged commit ee59c13 into jekyll:master Sep 8, 2014
parkr added a commit that referenced this pull request Sep 8, 2014
@akoeplinger akoeplinger deleted the fix-github-url branch September 9, 2014 10:20
@jekyll jekyll locked and limited conversation to collaborators Feb 27, 2017
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.

site.github.url may cause builds to fail—and is undocumented
4 participants