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

Fix missing env in maven docker context generator #1152

Closed
wants to merge 1 commit into from

Conversation

TadCordle
Copy link
Contributor

Again, changes for Maven 0.9.13 patch (skipping 0.9.12 to stay even with the Gradle plugin). Will close and re-open with the release PR after approval.

@TadCordle TadCordle requested a review from a team October 16, 2018 16:27
@@ -139,7 +139,7 @@ private static String mapToDockerfileString(Map<String, String> map, String comm
@Nullable private String baseImage;
private List<String> entrypoint = Collections.emptyList();
private List<String> javaArguments = Collections.emptyList();
private Map<String, String> environment = Collections.emptyMap();
@Nullable private Map<String, String> environment;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we have this be consistent with the others and not be nullable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

entrypoint and javaArguments actually become nullable in a future change, I think.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah right. I think if we make environment nullable, we should make exposedPorts and labels nullable too but that change can go in the post-release PR.

@TadCordle
Copy link
Contributor Author

#1153

@TadCordle TadCordle closed this Oct 16, 2018
@TadCordle TadCordle deleted the fix-docker-context-maven branch October 16, 2018 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants