Delete key from config instead of resetting value

algolia / jekyll-algolia

Add fast and relevant search to your Jekyll site

https://community.algolia.com/jekyll-algolia/

MIT License

214 stars 35 forks source link

Delete key from config instead of resetting value #126

Closed ashmaroli closed 5 years ago

ashmaroli commented 5 years ago

Closes #121

Semantically, deleting a key from a hash is the same as resetting it to nil as config[key] returns the same value in both cases. However, there's a chance that the config hash is traversed using Hash#fetch which returns a given default value or calls a given block only if the said key doesn't exist in the hash. This results in differing behavior.

Hence, deleting a config key is better than resetting it to nil.

An added benefit is that deleting a key frees allocated memory.

borisschapira commented 5 years ago

Hello there! Any plan on merging this? Would be useful to some of us 👍

nhoizey commented 5 years ago

I can confirm this PR works and fixes #121 for me

Ping @pixelastic

ashmaroli commented 5 years ago

@redox You're welcome. Out of curiosity, what exactly did you mean by LBTM?

When I first saw it, my brain registered it as a typo, but then expanded it to Looks Bad To Me (opposite of LGTM: Looks Good To Me). Gosh! :smile: Google search returned Laughing Behind The Monitor among other expansions irrelevant to the current context...

redox commented 5 years ago

Out of curiosity, what exactly did you mean by LBTM?

Ouch sorry, typo :) LGTM