search

RediSearch / redisearch-go

Go client for RediSearch
https://redisearch.io
BSD 3-Clause "New" or "Revised" License
298 stars 66 forks source link

Replace indexName parameter with Client.name in client #60

Closed dengliming closed 4 years ago

codecov[bot] commented 4 years ago

Codecov Report

Merging #60 into master will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #60   +/-   ##
=======================================
  Coverage   72.74%   72.74%           
=======================================
  Files          12       12           
  Lines        1108     1108           
=======================================
  Hits          806      806           
  Misses        239      239           
  Partials       63       63
Impacted Files Coverage Δ
redisearch/client.go 79.66% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 76b1c66...87a9bf4. Read the comment docs.

filipecosta90 commented 4 years ago

hi there @dengliming , I believe that we should avoid at all cost to include breaking changes on the methods signatures. IMHO we should not break backwards compatibility and potentially break user's code with this addition. If we think that not passing the index name is a must then we should include additional methods. what do you think?

dengliming commented 4 years ago

@filipecosta90 You're absolutely right, I'm sorry I didn't notice these methods was already in release version. And I think not passing the index name would be better.

filipecosta90 commented 4 years ago

hi there @dengliming, closing this one since as per our discussion above I believe we agree in not breaking the client