search

itstommymorgan / asari

a Ruby wrapper for AWS CloudSearch.
52 stars 51 forks source link

Added Facet Support #20

Open cbilgili opened 10 years ago

jwarchol commented 10 years ago

@duwanis Is there anything I can do to help get this PR reviewed? Facet support would be a very nice addition to Asari!

cbilgili commented 10 years ago

There was problem so I supposed it because of httparty. I had added to test it. But it was unnecessary, I have removed it. I will be happy if you test it and merge it with gem.

jwarchol commented 10 years ago

Sweet, thanks @cbilgili! Yesterday I made those same changes to my fork and have been testing it on our staging server. I'll let you and @duwanis know if I find any problems. Hopefully a bit of real-world use will prove this PR works well and can be merged.

itstommymorgan commented 10 years ago

Hey @jwarchol and @cbilgili -

Sorry that this PR has kind of sat inactive for a while - I clearly haven't been successful lately at making the time necessary for maintaining my projects. I'll get this PR reviewed early next week and then we can talk about merging.

jwarchol commented 10 years ago

Thanks @duwanis! Appreciate you taking the time to comment and I hope things work out and you're able to review this PR. Totally understand you're busy and grateful for when you do have time to help with OSS stuff.

(love your blog btw)

cbilgili commented 10 years ago

Thanks @jwarchol ! I can make revisions if necessary!

lgleasain commented 10 years ago

Hey guys,

I'm helping @wellbredgrapefruit with doing pull requests etc. on this this. I like the idea of adding facets and we always appreciate pull requests.

As I write this, if you look at the current unit test code coverage for the project it is at 100%. There is also a broken window in the form of 1 broken unit test on master that I am in the process of fixing along with adding in Travis-CI support. Can you add some unit tests to this so that subsequent changes don't accidentally break this? The implementation code looks good, and once we have some good specs I'll bring this in.

seban commented 10 years ago

@cbilgili @jwarchol can you add some unit tests to this PR, so @lgleasain can merge it? It would be nice to use it in official repo.