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

Added support for facet range queries #57

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

shiftyp
Copy link

@shiftyp shiftyp commented Apr 15, 2013

I've updated the Query.prototype.facet method to allow for a "range" option to specify a facet range query.

@shiftyp
Copy link
Author

shiftyp commented Apr 15, 2013

I still need to update the tests, I was hoping for some help in generating the appropriate response in line with whatever your Solr test setup is.

@lbdremy
Copy link
Owner

lbdremy commented Aug 15, 2013

Hello @shiftyp,

Firstly, I am really sorry I completely forgot/missed your pull request.

Secondly, I would be really happy to merge your code and publish it in the next release. I will do this really soon like by the end of the week. Anyway if later I will publish your changes in another release.

I just need you to add a couple things in order to merge it:

  • Add the JSDoc for the facet by range API and same thing for the stats stuff
  • Write a test for each new features, so two tests here
  • Extra cool: Add an example for each features in the folder examples/ in its own file

For the tests, I use the configuration that you can find here https://github.com/lbdremy/solr-node-client/tree/v0.3.x/test/materials when I test against Solr, on the latest version 3.
Otherwise the facet by range queries test should be written in this file https://github.com/lbdremy/solr-node-client/blob/master/test/test-facet.js. For the stats feature you can create a new file will look like something a bit like this test https://github.com/lbdremy/solr-node-client/blob/master/test/test-mlt.js. If you have other questions let me know, I am always glad to respond.

By the way, I really appreciate that you committed your changes with the same formatting like I do, thanks 👍 !

@lbdremy
Copy link
Owner

lbdremy commented Aug 15, 2013

ref #56

@lbdremy
Copy link
Owner

lbdremy commented Oct 2, 2013

Hi @shiftyp,

What's up?

@shiftyp
Copy link
Author

shiftyp commented Nov 26, 2013

Hey, sorry I've been out of communication. I'll take care of this as soon as I get a chance, which will be soon.

@lbdremy
Copy link
Owner

lbdremy commented Nov 27, 2013

No worries, looking forward.

@lbdremy lbdremy removed the v0.2.x label Aug 17, 2014
@luketaverne
Copy link
Collaborator

@shiftyp, still there? If I don't hear from you, I'll look into merging this and writing the tests/ examples myself.

@luketaverne luketaverne self-assigned this Oct 2, 2015
@nicolasembleton
Copy link
Collaborator

This looks like a nice feature indeed. @luketaverne I think we should go ahead and merge. I can help here if you need.

@laukaichung
Copy link

@nicolasembleton please go ahead and merge.

@tokyoben
Copy link

tokyoben commented Aug 3, 2017

Is this going to be merged?

@shiftyp
Copy link
Author

shiftyp commented Dec 22, 2017

Wow, very old unfinished contribution flashback 😵. I may actually have some time coming up in January where I could work on this. Anyone else interested in this issue though can feel free to pick up in the meantime and address the conflicts and the three tasks outstanding:

  • Add the JSDoc for the facet by range API and same thing for the stats stuff
  • Write a test for each new features, so two tests here
  • Extra cool: Add an example for each features in the folder examples/ in its own file

@kibertoad
Copy link
Collaborator

@shiftyp Any chance you could finish this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants