#10775 Implement Valkey module#11101
Conversation
wwisser
changed the title
|
|
||
| @Override | ||
| public void start() { | ||
| List<String> command = new ArrayList<>(); |
There was a problem hiding this comment.
nit: This should be final.
There was a problem hiding this comment.
I'm not a fan of this because excessive usage of final in Java clutters the code and worsens readability. It's pretty uncommon and I don't see a presence of this convention in the codebase either.
I'm happy to discuss it, but contribution guidelines state we should adapt to code base standards.
|
|
||
| @Test | ||
| void shouldMountValkeyConfigToContainer() throws Exception { | ||
| Path configFile = Paths.get(getClass().getResource("/valkey.conf").toURI()); |
There was a problem hiding this comment.
Variables that do not change state after declaration should be explicitly marked as final.
…cution; utilize AutoCloseable iface in tests; make use of StringUtils.isNotEmpty for preconditions
|
Hi @wwisser, I would suggest next time to wait until we are aiming to have this module. As mentioned in our guidelines, this should be discussed first because we know this requires time and effort. Testcontainers modules are best-effort modules, we do not have the whole knowledge of all those technologies, besides that it will add maintenance on our side. So, we are not going to host the module. Thanks for your contribution, anyway. |
3 participants
As requested in #10775, this PR will add support for Valkey testcontainers similar as we already have them for Node, Go & Rust.
I've included most of the features that currently supporting languages have such as:
Ready to accept connections