Update Search client to Search API v3

twingly / twingly-search-api-ruby

:gem: Twingly Blog Search API in Ruby

https://developer.twingly.com/

MIT License

4 stars 0 forks source link

Update Search client to Search API v3 #65

Closed roback closed 7 years ago

roback commented 7 years ago

Prepare the Search API client for Blog Search API v3

Todo

[x] Make Query#pattern and Post backwards compatible (36788069d6c409b40816895422fb57cb6cf5ac70)
[x] Read through the documentation and make sure it's parsed correctly by yardoc (13b0fe2dcb2bc2f074e6f9abc9401e5c965a3934)
[x] Deprecate Query#language (ed6909f23178542a4d00fa5343745254c1cec3b3)
[x] Deprecate Post#outlinks (ab41cea55502355c315ea24f18304c6ce037fcac)

jage commented 7 years ago

Should we update the example to use published_at? https://github.com/twingly/twingly-search-api-ruby/blob/f9f95eaefb1699a1d89c5bd6af9cfccf9199b7f2/examples/find_all_posts_mentioning_github.rb#L28

roback commented 7 years ago

The reason there were so many lines removed here (+1,148 −24,223) is that the fixture spec/fixtures/valid_result.xml was removed. I didn't see any point of having it since there already is a smaller fixture with a valid API result in the repo (spec/fixtures/minimal_valid_result.xml).

jage commented 7 years ago

Just a thought, should we keep pattern around, but deprecated?

I feel like we are "in between" a major release (breaking changes to search_query). But we have some backwards compatible fixes for all the changed attributes ... so in one way this isn't breaking anything (new methods should be allowed to classes)?

I'm thinking this could be a minor version bump if we keep pattern as a deprecated method.

WDYT @roback ?

roback commented 7 years ago

Just a thought, should we keep pattern around, but deprecated?

That is already the case (https://github.com/twingly/twingly-search-api-ruby/pull/65/files#diff-085b0a7f794a0d2f3248b1ff247629eeR15) :) I just switched to using def instead of attr_accessor so that I was able to print a warning message when it is used.

I'm thinking this could be a minor version bump if we keep pattern as a deprecated method.

I thought these changes would break something at first, but you're right, it seems like there are no breaking changes.

jage commented 7 years ago

That is already the case (https://github.com/twingly/twingly-search-api-ruby/pull/65/files#diff-085b0a7f794a0d2f3248b1ff247629eeR15) :) I just switched to using def instead of attr_accessor so that I was able to print a warning message when it is used.

Oh, missed that, smart. :)

jage commented 7 years ago

I'm thinking like this:

Minor release now with more functionality but some deprecated methods
Major release in the future which will remove these methods

roback commented 7 years ago

Should we deprecate Query#language=? I'm thinking it should be specified in the search query.

:+1: I didn't think about language at all, but yes, it would be a good idea to deprecate it at the same time as well.

I think we should "always" keep start/end-time though since it's very helpful when iterating over many results.

Exactly my thinking :)

roback commented 7 years ago

I'm thinking this could be a minor version bump if we keep pattern as a deprecated method.

Post#outlinks should be deprecated as well, then I think we can use a minor version.