search

symfony-cmf / search-bundle

UNMAINTAINED - search Bundle for the Symfony CMF project integrating with LiipSearchBundle
http://cmf.symfony.com/
8 stars 10 forks source link

define limit for search results #28

Closed cristianbuj closed 9 years ago

cristianbuj commented 9 years ago

doc PR https://github.com/symfony-cmf/symfony-cmf-docs/pull/608

lsmith77 commented 9 years ago

looks good to me ..

@dbu if its fine for you as well, I can merge and tag a new release. I guess this should also be added to the google provider, though I guess that one might need some work anyway?

dbu commented 9 years ago

i think google had a default max results setting with the old dysfunct api. but that code is in LiipSearchBundle. google search api was started to be fixed in https://github.com/liip/LiipSearchBundle/pull/12 (that pr stalled, but actually commits where added 2 weeks ago...)

i think we have been discussing if we can merge this and LiipSearchBundle or otherwise simplify things. a max result configuration would be something that should affect every search provider implementation. but for the time being, i am fine to have this here for now, as it solves a real need. merging the two bundles or other cleanups are very likely to bump us to a version 2.0 anyways.

dbu commented 9 years ago

thanks! can you please fix the formatting and prepare a pull request against the search config reference? http://symfony.com/doc/master/cmf/bundles/search/configuration.html

dbu commented 9 years ago

oh, and if you can squash your commits after the codestyle fix, that would be nice. (do the new commit, then git rebase -i HEAD~3 and if it looks right, git push -f)

levuro commented 9 years ago

@cristianbuj - please do this once you are back from vacation :)

dbu commented 9 years ago

apart from the phpdoc comment, this is ready to merge now. thanks for cleaning up the phpdoc mistakes we had in the controller.

cristianbuj commented 9 years ago

Sorry, I missed it, was added by PhpStorm. It's removed now.

dbu commented 9 years ago

thanks a lot for the contribution!

lsmith77 commented 9 years ago

@dbu do you want to include this change in a 1.1.1 release or was it intended to wait for 1.2.0?

dbu commented 9 years ago

created 1.1.1