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

Use Liquid::Drops instead of Hashes in #to_liquid #4277

Merged
merged 15 commits into from Dec 26, 2015
Merged

Conversation

parkr
Copy link
Member

@parkr parkr commented Dec 22, 2015

This requires test updates but it compiles all the (rudimentary) sites on my machine.

Fixes #3224.

@jekyll/core, please review.

The properties of Liquid::Drops are only evaluated when they're asked for
and therefore save computation time. This prevents a lot of GC time cleaning
up objects that are not needed, because they're not created unless requested.
Additionally, this saves time for actual computation of those values because
they can be computed only if needed.

It's funny how much it helps when you only do what is needed. Far less overhead.
@parkr
Copy link
Member Author

parkr commented Dec 22, 2015

@envygeeks How do I run this new code through Rubocop?

@envygeeks
Copy link
Contributor

You're on OS X so you'll need to install Docker: https://www.docker.com/docker-toolbox (the preferred Method for OS X.) If that doesn't work I'll create a bot that does this all automatically and posts it to the pull request and flags it as needs-manual-review.

@alfredxing
Copy link
Member

👍 Looks good to me so far!

@parkr
Copy link
Member Author

parkr commented Dec 23, 2015

Down to 3 failures & 6 errors on the test spec. Cucumber specs run and all pass, though there's a strange issue in the reporter on travis. Maybe a cached version or something that is causing issues.

@parkr
Copy link
Member Author

parkr commented Dec 24, 2015

@jekyll/core I need to finish writing tests for these drops but otherwise we should be good to go 👍

Please review this PR.

parkr added a commit that referenced this pull request Dec 26, 2015
@parkr parkr merged commit ec25dde into master Dec 26, 2015
@parkr parkr deleted the document-drops branch December 26, 2015 02:32
parkr added a commit that referenced this pull request Dec 26, 2015
parkr added a commit that referenced this pull request Dec 26, 2015
@alfredxing
Copy link
Member

👍 🎉

parkr added a commit that referenced this pull request Dec 26, 2015
@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.

Use Liquid::Drop to liberate and speed up Jekyll.
4 participants