Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove @Nullable annotations and dependency on com.google.code.findbugs:jsr305 #920

Merged
merged 3 commits into from
Nov 5, 2018

Conversation

dbyron0
Copy link
Contributor

@dbyron0 dbyron0 commented Oct 15, 2018

Another (in addition to #913) way to address #912.

@kiview
Copy link
Member

kiview commented Oct 15, 2018

This is the only location where we use @Nullable?

@dbyron0
Copy link
Contributor Author

dbyron0 commented Oct 15, 2018

Apparently not:

$ GIT_PAGER="" git grep Nullable
core/src/main/java/org/testcontainers/containers/Container.java:        Optional<String> oldValue = Optional.ofNullable(getEnvMap().get(key));
core/src/main/java/org/testcontainers/containers/GenericContainer.java:import org.jetbrains.annotations.Nullable;
core/src/main/java/org/testcontainers/containers/GenericContainer.java:    @Nullable
core/src/main/java/org/testcontainers/containers/PortForwardingContainer.java:        return Optional.ofNullable(container)
core/src/main/java/org/testcontainers/dockerclient/DockerClientConfigUtils.java:            .ofNullable(DockerClientFactory.instance().runInsideDocker(
core/src/main/java/org/testcontainers/dockerclient/DockerClientProviderStrategy.java:import org.jetbrains.annotations.Nullable;
core/src/main/java/org/testcontainers/dockerclient/DockerClientProviderStrategy.java:                        @Nullable String throwableMessage = e.getMessage();
core/src/main/java/org/testcontainers/dockerclient/DockerClientProviderStrategy.java:                        @Nullable String rootCauseMessage = rootCause.getMessage();
core/src/main/java/org/testcontainers/utility/AuditLogger.java:import org.jetbrains.annotations.Nullable;
core/src/main/java/org/testcontainers/utility/AuditLogger.java:                             @Nullable String image,
core/src/main/java/org/testcontainers/utility/AuditLogger.java:                             @Nullable String containerId,
core/src/main/java/org/testcontainers/utility/AuditLogger.java:                             @Nullable String image,
core/src/main/java/org/testcontainers/utility/AuditLogger.java:                             @Nullable String containerId,
core/src/main/java/org/testcontainers/utility/AuditLogger.java:                             @Nullable Exception e) {
core/src/main/java/org/testcontainers/utility/AuditLogger.java:                                    @Nullable List<String> env) {
modules/cassandra/src/main/java/org/testcontainers/containers/CassandraContainer.java:        Optional.ofNullable(resourceLocation)
modules/jdbc/src/main/java/org/testcontainers/jdbc/ConnectionUrl.java:        imageTag = Optional.ofNullable(urlMatcher.group(3));
modules/jdbc/src/main/java/org/testcontainers/jdbc/ConnectionUrl.java:            databasePort = Optional.ofNullable(dbInstanceMatcher.group(3)).map(value -> Integer.valueOf(value));
modules/jdbc/src/main/java/org/testcontainers/jdbc/ConnectionUrl.java:                                                Optional.ofNullable(urlMatcher.group(5)).orElse("")));
modules/jdbc/src/main/java/org/testcontainers/jdbc/ConnectionUrl.java:        initScriptPath = Optional.ofNullable(containerParameters.get("TC_INITSCRIPT"));
modules/selenium/src/main/java/org/testcontainers/containers/BrowserWebDriverContainer.java:import org.jetbrains.annotations.Nullable;
modules/selenium/src/main/java/org/testcontainers/containers/BrowserWebDriverContainer.java:    @Nullable
modules/selenium/src/main/java/org/testcontainers/containers/BrowserWebDriverContainer.java:    @Nullable

but I guess it's the only place that uses javax.annotation.Nullable. Looks like the rest use org.jetbrains.annotations.Nullable

@dbyron0
Copy link
Contributor Author

dbyron0 commented Oct 15, 2018

For what it's worth, the test failures (timeouts) look unrelated to this change.

@@ -233,9 +232,8 @@ public void put(InputStream body, com.github.dockerjava.core.MediaType mediaType
execute(request).close();
}

protected RequestBody toRequestBody(InputStream body, @Nullable String mediaType) {
protected RequestBody toRequestBody(InputStream body, String mediaType) {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should replace this with org.jetbrains.annotations.Nullable? At least this PR would get us to a consistent state, from which point we can consider whether it's worth keeping the jetbrains library in place!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure: 1f5b77c

@kiview
Copy link
Member

kiview commented Nov 5, 2018

I just merged with master, regarding the comment of @rnorth, I think we can merge this.

@kiview kiview added this to the next milestone Nov 5, 2018
@kiview kiview merged commit 043e90a into testcontainers:master Nov 5, 2018
@kiview kiview mentioned this pull request Nov 5, 2018
@dbyron0 dbyron0 deleted the removeNullable branch November 5, 2018 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants