#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.
modules/valkey/src/main/java/org/testcontainers/valkey/ValkeyContainer.java
Outdated
Show resolved
Hide resolved
modules/valkey/src/main/java/org/testcontainers/valkey/ValkeyContainer.java
Show resolved
Hide resolved
modules/valkey/src/test/java/org/testcontainers/valkey/ValkeyContainerTest.java
Outdated
Show resolved
Hide resolved
modules/valkey/src/test/java/org/testcontainers/valkey/ValkeyContainerTest.java
Outdated
Show resolved
Hide resolved
|
|
||
| @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.
modules/valkey/src/test/java/org/testcontainers/valkey/ValkeyContainerTest.java
Outdated
Show resolved
Hide resolved
…cution; utilize AutoCloseable iface in tests; make use of StringUtils.isNotEmpty for preconditions
modules/valkey/src/main/java/org/testcontainers/valkey/ValkeyContainer.java
Outdated
Show resolved
Hide resolved
modules/valkey/src/main/java/org/testcontainers/valkey/ValkeyContainer.java
Outdated
Show resolved
Hide resolved
modules/valkey/src/main/java/org/testcontainers/valkey/ValkeyContainer.java
Show resolved
Hide resolved
|
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