Skip to content

Added support for facet range queries#57

Open
shiftyp wants to merge 4 commits into
lbdremy:masterfrom
shiftyp:master
Open

Added support for facet range queries#57
shiftyp wants to merge 4 commits into
lbdremy:masterfrom
shiftyp:master

Conversation

@shiftyp

@shiftyp shiftyp commented Apr 15, 2013

Copy link
Copy Markdown

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

@shiftyp

shiftyp commented Apr 15, 2013

Copy link
Copy Markdown
Author

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

lbdremy commented Aug 15, 2013

Copy link
Copy Markdown
Owner

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

lbdremy commented Aug 15, 2013

Copy link
Copy Markdown
Owner

ref #56

@lbdremy

lbdremy commented Oct 2, 2013

Copy link
Copy Markdown
Owner

Hi @shiftyp,

What's up?

@shiftyp

shiftyp commented Nov 26, 2013

Copy link
Copy Markdown
Author

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

lbdremy commented Nov 27, 2013

Copy link
Copy Markdown
Owner

No worries, looking forward.

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

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown

@nicolasembleton please go ahead and merge.

@tokyoben

tokyoben commented Aug 3, 2017

Copy link
Copy Markdown

Is this going to be merged?

@shiftyp

shiftyp commented Dec 22, 2017

Copy link
Copy Markdown
Author

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
Copy Markdown
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