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

Do not cache the drop payload for SeoTag #306

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@ashmaroli
Copy link
Member

ashmaroli commented Oct 3, 2018

For compatibility with Jekyll 4.0.. and current jekyll:master..

@DirtyF

DirtyF approved these changes Oct 3, 2018

@DirtyF DirtyF requested review from benbalter and pathawks Oct 3, 2018

@yyyc514

This comment has been minimized.

Copy link

yyyc514 commented Dec 22, 2018

Since we have the conditional is there a reason this is hung and not getting merged into master?

@ashmaroli

This comment has been minimized.

Copy link
Member Author

ashmaroli commented Dec 22, 2018

is there a reason this is hung

Perhaps the maintainers are waiting for Jekyll core to finalize whether the said "caching Liquid templates" enhancement would be included in the final release in order to avoid unnecessary code-churn...

Additionally, the conditional is a hack and as of now, the line
if context.registers[:site].liquid_renderer.respond_to?(:cache) would be executed for every call to the drop method, affecting performance in sites not willing to upgrade to Jekyll 4.x..

Lots of decisions to take..

@yyyc514

This comment has been minimized.

Copy link

yyyc514 commented Dec 23, 2018

Good points. I don't think the if/else would be that slow, but I suppose you can argue it'd be potentially slower than nothing at all that we're currently doing. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment