Skip to content
This repository has been archived by the owner on Feb 8, 2019. It is now read-only.

GEARPUMP-215 Gearpump Redis Integration - RedisSink #93

Closed
wants to merge 1 commit into from

Conversation

darionyaphet
Copy link
Contributor

@codecov-io
Copy link

codecov-io commented Oct 10, 2016

Current coverage is 71.34% (diff: 100%)

Merging #93 into master will decrease coverage by 0.21%

@@             master        #93   diff @@
==========================================
  Files           186        186          
  Lines          5985       5985          
  Methods        5464       5464          
  Messages          0          0          
  Branches        521        521          
==========================================
- Hits           4283       4270    -13   
- Misses         1702       1715    +13   
  Partials          0          0          

Sunburst

Powered by Codecov. Last update 9b87945...1bfab21

* @param database
* @param password
*/
class RedisStorage(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better name it RedisSink since it extends DataSink interface

import redis.clients.jedis.Protocol.{DEFAULT_DATABASE, DEFAULT_HOST, DEFAULT_PORT, DEFAULT_TIMEOUT}

/**
* Save message in Redis Instance
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please fix the comment style. If you are using Intellij, please go to Editor => Code Style => Scala, switch to Wrapping and Braces tab, click on Keep when reformatting and check Comment at first column

@manuzhang
Copy link
Contributor

looks good overall despite minor naming and style things

@darionyaphet darionyaphet changed the title GEARPUMP-215 Gearpump Redis Integration - RedisStorage GEARPUMP-215 Gearpump Redis Integration - RedisSink Oct 10, 2016
@manuzhang
Copy link
Contributor

@darionyaphet +1, thanks for your contribution. Please keep the good work.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants