Bing web search client

[benglass]

[benglass]

@dbu If you would like to take a look at this PR before I clean it up and release a version based on it let me know

[dbu]

could also be offset after last result. lets remove the fixme and just have a comment that there are no results and no error.

Am 24.04.2017 4:01 nachm. schrieb "Ben Glassman" [email protected]:

@benglass commented on this pull request.

In SearchClient/BingWebSearchAdapter.php https://github.com/liip/LiipSearchBundle/pull/30#discussion_r112953221:

•    $this->restrictToSites = $restrictToSites;
  

•    $this->maxTimeoutMs = $maxTimeoutMs;
  

•    $this->maxRetries = $maxRetries;
  

•    $this->retryDelayMs = $retryDelayMs;
  

• }
• /**

• * Get search results
  

• *
  

• * {@inheritdoc}
  

• */
  

• public function getSlice($offset, $length)
• {

•    $results = $this->executeSearch($offset, $length);
  

•    // FIXME: Hopefully this doesnt hide errors
  

@dbu https://github.com/dbu It seems this is just indicative that there were no results returned and not that there is an error. The code already handles HTTP non-200 status codes which appears to be how bing's api handles errors and not with the content of the response so I believe this is actually the correct behavior. This is less of a fixme and more of a note that I am uncertain if there is a scenario where this might indicate and error instead of 'no results'.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/liip/LiipSearchBundle/pull/30#discussion_r112953221, or mute the thread https://github.com/notifications/unsubscribe-auth/AAErINADyE9_ynOIdJ90q-m94yXn2-aWks5rzKs8gaJpZM4NEo-U .

[benglass]

@dbu I made the change you requested to the FIXME and squashed the commits. Good to merge?

Is your normal release process just to do "git tag 3.0.0" ?

[dbu]

i would also change the version alias of the master branch before merging: https://github.com/liip/LiipSearchBundle/blob/581bd134a5b77662a93c9e665f75dccd345e67b5/composer.json#L32

to release i go to the project start page, there is a "releases" section. there you can see changes since the last tag to verify what you are releasing (check for BC breaks and stuff) and then "Draft a new release". i use the tag name as release name and try to list the most important changes in the description.

i would recommend to do another PR to delete the google search once this PR is merged, before you tag 3.0. removing code and configuration is a BC break and should only happen in major version changes.

[benglass]

@dbu You had suggested that I remove the google code for the 3.0 branch. I take your point that its a BC change (at this moment in time) and therefore only suitable for a major version release but as it stands now google search continues to work. It wont completely stop working until April 1, 2018

For this reason, I think I prefer to merge in bing support and release as 3.0 and then remove google next year when it is no longer a service. At that point its not really a BC break in my eyes because the service doesnt exist anymore and cannot be used, but we could also release a 4.0 at that point.

In summary, I'd like to release 3.0 now with bing support and google site search left in because it continues to exist until April 2018.

Thoughts?

[dbu]

hi @benglass

@lsmith77 and i are currently archiving inactive repositories in the liip organization. this made me stumble over this one. i suggest the following: you take this branch to build a Benglass\SearchBundle (or your company namespace or whatever fits best) and replace the namespace to be yours. if you mention that this started from the LiipSearchBundle, we appreciate it of course.

that will be easier to manage than keeping this in the liip organziation when we are not active on it at all. also gives you better credit for your work ;-) if you want to do that, please tell me the new place and i abandon the LiipSearchBundle in favor of yours, on packagist and here in github.

[dbu]

I will archive this repository and abandon it on packagist. If you release a Bing bundle, please tell me and i will link to it.