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

Jump to bottom

Memoize fallback_data for Drop #7728

Merged
merged 1 commit into from Aug 4, 2019
Merged

Memoize fallback_data for Drop #7728

merged 1 commit into from Aug 4, 2019

Conversation

ashmaroli
Copy link
Member

To avoid generating a new Hash object on every call

@DirtyF DirtyF requested a review from a team June 28, 2019 15:51
@DirtyF
Copy link
Member

DirtyF commented Jun 28, 2019

Any possible side effect to memoization here?

@ashmaroli
Copy link
Member Author

No. It is safe. Another drop with a memoized fallback:

jekyll/lib/jekyll/drops/unified_payload_drop.rb

Lines 19 to 23 in 2265e82

private
def fallback_data
@fallback_data ||= {}
end

@DirtyF
Copy link
Member

DirtyF commented Aug 4, 2019

@jekyllbot: merge +dev

@jekyllbot jekyllbot merged commit a0c3a6b into jekyll:master Aug 4, 2019
jekyllbot added a commit that referenced this pull request Aug 4, 2019
@jekyllbot
Update history to reflect merge of #7728 [ci skip]
2736589
@ashmaroli ashmaroli deleted the drop-fallback-data branch August 5, 2019 04:13
@jekyll jekyll locked and limited conversation to collaborators Aug 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@DirtyF DirtyF DirtyF approved these changes

@parkr parkr parkr approved these changes

Assignees
No one assigned
Labels
fix frozen-due-to-age internal
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@ashmaroli @DirtyF @parkr @jekyllbot