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

Fix compatibility with Jekyll >= 1.1.0 #14

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

IQAndreas
Copy link
Contributor

Adds an additional argument added in a newer Jekyll version. Luckily,
since the value is optional, this plugin is still compatible with older
versions of Jekyll.

Fixes #13

Adds an additional argument added in a newer Jekyll version. Luckily,
since the value is optional, this plugin is still compatible with older
versions of Jekyll.

Fixes mpalmer#13
@parkr
Copy link

parkr commented Oct 20, 2013

You won't get any attributes if you pass in nil.

@parkr
Copy link

parkr commented Oct 20, 2013

#15 should do it.

@IQAndreas
Copy link
Contributor Author

You won't get any attributes if you pass in nil.

Actually, you will, since Jekyll's version of to_liquid looks like this:

(attrs || self.class::ATTRIBUTES_FOR_LIQUID)

@IQAndreas
Copy link
Contributor Author

#15 should do it.

Actually, your code isn't compatible with previous versions of Jekyll (and for reference, Octopress still uses Jekyll 0.12.0).

I added a fix for reverse-compatibility with previous versions in my fork, but had forgotten to add those fixes to this pull request.

See what I mean by "not compatible with other versions"? Damn monkey-patching dynamic languages.

@parkr
Copy link

parkr commented Oct 20, 2013

@IQAndreas That's totally fine, as this is a plugin for Jekyll. My fix happens to be for the latest version of Jekyll, as the only rational assumption for me to make is that this plugin is compatible with Jekyll v1.x, as it's been out for many months now. Octopress is another story and I didn't consider it here for the reason you stated.

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.

Plugin crashes for Jekyll >= 1.1.0
2 participants