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

refactor #as_liquid #3158

Merged
merged 4 commits into from Dec 9, 2014

Conversation

Projects
None yet
3 participants
@afeld
Contributor

afeld commented Nov 27, 2014

Building off of #3154. Like how the refactored version is longer? A little less clear, too, but more robust.

fail

@parkr Not sure if the github-metadata use case you mentioned can be covered by this method. From what I'm seeing, the procs require varying numbers of plugin-specific arguments, so not sure if there's anything that can be done here.

@parkr parkr added the refactor label Dec 1, 2014

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Dec 7, 2014

Member

Why go with if instead of case here?

Member

parkr commented Dec 7, 2014

Why go with if instead of case here?

@afeld

This comment has been minimized.

Show comment
Hide comment
@afeld

afeld Dec 9, 2014

Contributor

The third check is a respond_to?, which can't be done in a case statement. Not a big deal: changed it back.

Contributor

afeld commented Dec 9, 2014

The third check is a respond_to?, which can't be done in a case statement. Not a big deal: changed it back.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Dec 9, 2014

Member

Dope. 👍 Thanks!

Member

parkr commented Dec 9, 2014

Dope. 👍 Thanks!

@parkr parkr merged commit 49115db into jekyll:master Dec 9, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

parkr added a commit that referenced this pull request Dec 9, 2014

@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.