Skip to content

Emulate Hashie::Mash key query method #41

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

Closed
wants to merge 1 commit into from

Conversation

legiar
Copy link

@legiar legiar commented Mar 14, 2014

Hashie::Mash have perfectly method to query key exists (ended with question sign).
For dry usage result from ElasticSearch with different data we can also use this method.

Hashie::Mash have perfectly method to query key exists (ended with question sign).
For dry usage result from ElasticSearch with different data we can also use this method.
@karmi
Copy link
Contributor

karmi commented Mar 14, 2014

Hi, nice catch!, that should indeed work. Two notes:

  1. I'd like to see it implemented with end_with? for better clarity, and
  2. We definitely need a test for this in response_result_test.rb

Also, please check the contributor guidelines and sign the CLA if you haven't already.

@karmi
Copy link
Contributor

karmi commented Mar 19, 2014

@legiar What do you think about the proposed tweaks?

@karmi karmi added the waiting label Mar 19, 2014
@legiar
Copy link
Author

legiar commented Mar 19, 2014

Yep. But I do small changes (I also have some additional ideas) on ends of
this week.

On Wed, Mar 19, 2014 at 6:53 PM, Karel Minarik notifications@github.comwrote:

@legiar https://github.com/legiar What do you think about the proposed
tweaks?


Reply to this email directly or view it on GitHubhttps://github.com//pull/41#issuecomment-38077020
.

Mikhail Mikhaliov, Ruby engineer | Cogniance, Inc | www.cogniance.com
E-mail: mmikhaliov@cogniance.com | Mob.: +38 050 440 36 06 | Skype:
mikhail.mikhaliov

emptyflask added a commit to emptyflask/elasticsearch-rails that referenced this pull request Apr 3, 2014
(that pull request addressed this issue as well)
@emptyflask
Copy link
Contributor

I didn't realize this pull request existed when I submitted #64, but that one has test coverage, and now uses String#end_with?

@karmi
Copy link
Contributor

karmi commented Apr 3, 2014

@emptyflask Yeah, these are functionally equivalent. @legiar What do you think?

I need to wrap something biggish up, I'll return to processing the PRs shortly...

karmi added a commit that referenced this pull request Apr 17, 2014
…t.foo?`) implementation and tests

Closes ##70

Related: #41, #64
@karmi
Copy link
Contributor

karmi commented Apr 17, 2014

@legiar Ended up going with @emptyflask's patch from #64, please see the attached commit for full story.

@karmi karmi closed this Apr 17, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants