RedisBloom / JRedisBloom

Java Client for RedisBloom probabilistic module
https://redisbloom.io
BSD 2-Clause "Simplified" License
153 stars 33 forks source link

Implements Count-Min-Sketch RedisBloom Commands #29

Closed bsbodden closed 3 years ago

bsbodden commented 3 years ago
codecov[bot] commented 3 years ago

Codecov Report

Merging #29 (653b436) into master (9e372a9) will increase coverage by 4.99%. The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #29      +/-   ##
==========================================
+ Coverage   45.84%   50.83%   +4.99%     
==========================================
  Files           9       10       +1     
  Lines         517      299     -218     
  Branches       69       20      -49     
==========================================
- Hits          237      152      -85     
+ Misses        261      144     -117     
+ Partials       19        3      -16     
Impacted Files Coverage Δ
src/main/java/io/rebloom/client/Client.java 0.00% <ø> (-35.42%) :arrow_down:
...rc/main/java/io/rebloom/client/cms/CMSCommand.java 0.00% <0.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 9e372a9...653b436. Read the comment docs.

bsbodden commented 3 years ago

@bsbodden Could you please unformat the unchanged codes, specially in ClientTest.java? This would really help the review process.

Done. Thanks.

bsbodden commented 3 years ago

In addition to the individual comments, you should consider using two space characters instead of a tab character.

Yes, indeed. New machine, missed changing the tabs to spaces in the editor. But traditionally for Java is 4 spaces for indents.

sazzad16 commented 3 years ago

traditionally for Java is 4 spaces for indents

True, but 2 spaces for indents has also been a norm for a long time.

This project follows 2 spaces, so we should (try to) maintain that.

PS: Google uses 2 spaces.

bsbodden commented 3 years ago

traditionally for Java is 4 spaces for indents

True, but 2 spaces for indents has also been a norm for a long time.

This project follows 2 spaces, so we should (try to) maintain that.

PS: Google uses 2 spaces.

I'm fine with that, I do a lot of Ruby too so 2 spaces looks better IMO :-). We should probably have a formatting file for the projects themselves and contributors can just pull them n for their PRs

bsbodden commented 3 years ago

@sazzad16 rebased all my files at 2 space indentation.

ashtul commented 3 years ago

The code looks great! Thank you for adding it.