Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Mutate hash literals instead of duplicating them #417

Merged
merged 1 commit into from
Oct 7, 2020
Merged

Mutate hash literals instead of duplicating them #417

merged 1 commit into from
Oct 7, 2020

Conversation

ashmaroli
Copy link
Member

Prefer Hash#merge! over Hash#merge for temporary Hashes.
At a micro-level, the mutating method is faster as well.

Additionally, stash page["image"] in a local variable in ImageDrop so as to reduce calls to Jekyll::Drops::DocumentDrop#[]

@ashmaroli ashmaroli requested a review from a team October 5, 2020 16:50
parkr
parkr approved these changes Oct 7, 2020
View reviewed changes
Copy link
Member

@parkr parkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If tests pass, 👍

DirtyF
DirtyF approved these changes Oct 7, 2020
View reviewed changes
Copy link
Member

@DirtyF DirtyF left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What Parkr said 😄

@DirtyF
Copy link
Member

DirtyF commented Oct 7, 2020

@jekyll: merge + dev

@jekyllbot jekyllbot merged commit 76e062d into jekyll:master Oct 7, 2020
jekyllbot added a commit that referenced this pull request Oct 7, 2020
@jekyllbot
Update history to reflect merge of #417 [ci skip]
dac459f
@ashmaroli ashmaroli deleted the merge-hash-literals branch October 7, 2020 16:12
@jekyll jekyll locked and limited conversation to collaborators Oct 7, 2021
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Reviewers

@DirtyF DirtyF DirtyF approved these changes

@parkr parkr parkr approved these changes

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

Successfully merging this pull request may close these issues.
4 participants
@ashmaroli @DirtyF @parkr @jekyllbot