10-Enable Redis Ready Pattern to be set as part of Redis Exec Provider. #11
Conversation
|
|
||
| protected abstract Pattern redisReadyPattern(); | ||
|
|
||
| public abstract void setRedisReadyPat(Pattern redisReadyPat); |
There was a problem hiding this comment.
I'd like to have this set via the Builder pattern, and not be able to mutate this class. Could you remove this method, and set the Pattern via the Builders instead ?
| } | ||
|
|
||
| @Override | ||
| public void setRedisReadyPat(Pattern redisReadyPat) { |
There was a problem hiding this comment.
same as before, it should be set in the Builder and passed via the class constructor.
|
|
||
| private final Map<OsArchitecture, String> executables = Maps.newHashMap(); | ||
| private final Map<OsArchitecture, RedisExecutor> executables = Maps.newHashMap(); | ||
| public final static Pattern REDIS_READY_PATTERN = Pattern.compile( |
There was a problem hiding this comment.
More of nitpick than anything, but could you name it DEFAULT_REDIS_READY_PATTERN ?
That way we really convey that it's overridable.
|
|
||
| import java.util.regex.Pattern; | ||
|
|
||
| public class RedisExecutor { |
There was a problem hiding this comment.
The Executor part of the name make me think of java.util.concurrent.Executor, which lead to think it control how the process is run.
Could it be renamed to RedisExecutable (or something like that) ?
|
Any chances a publish a release binary soon? Thanks |
|
Test runs ok on my side, can the check reinitiated? thanks. |
|
I'll try to make a release this week, thanks for the contribution ! |
Issue #10 PR, please review.
The idea is to extend RedisExecProvider to hold the executable name and it's server ready pattern.