[MSHARED-969] Environment variable with null value#68
[MSHARED-969] Environment variable with null value#68MartinKanters merged 1 commit intoapache:masterfrom slawekjaranowski:fix-969
Conversation
src/test/java/org/apache/maven/shared/utils/cli/CommandLineUtilsTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/apache/maven/shared/utils/cli/CommandLineUtilsTest.java
Outdated
Show resolved
Hide resolved
|
@pzygielo thanks for review, I did fix |
mkarg
left a comment
mkarg
left a comment
There was a problem hiding this comment.
I did not talk about popularity neither about reflection, but solely about creating garbage needlessly. I doubt the race condition actually will happen here, as the variable is effectively final at that point.
| public void environmentVariableWithNullShouldNotBeAddedToList() { | ||
|
|
||
| Commandline commandline = new Commandline(); | ||
| commandline.addEnvironment("TEST_NULL_ENV", null); |
There was a problem hiding this comment.
With reard to my previous comments I'd like to ask the OS experts what sense it make to store null in an environment variable?
There was a problem hiding this comment.
As I wrote before Command Line object has local map of environment variable which are provided to Runtime.exec after translate this map to list of name=value. Translation is done in getEnvironmentVariables.
Please look at my last comment at jira and new test in last update.
MartinKanters
left a comment
MartinKanters
left a comment
There was a problem hiding this comment.
I agree with @slawekjaranowski and @elharo, this does fix potential bugs. I approve the changes. Thanks @slawekjaranowski!
|
Resolve #282 |
6 participants
No description provided.