Added support for interpolated variables #35
Conversation
pom.xml
Outdated
| <groupId>commons-beanutils</groupId> | ||
| <artifactId>commons-beanutils</artifactId> | ||
| <version>1.9.3</version> | ||
| </dependency> |
There was a problem hiding this comment.
@jglick @oleg-nenashev do you have any insight if these dependencies are safe to add?
´commons-beanutils´ is provided by core IIRC?
There was a problem hiding this comment.
Cannot recall offhand, but should be checked.
There was a problem hiding this comment.
Indeed jenkins-core already provides beanutils, I've removed the dependency.
pom.xml
Outdated
| <groupId>commons-beanutils</groupId> | ||
| <artifactId>commons-beanutils</artifactId> | ||
| <version>1.9.3</version> | ||
| </dependency> |
There was a problem hiding this comment.
Cannot recall offhand, but should be checked.
| @@ -82,6 +87,9 @@ | |||
| properties.load(sr); | |||
| } | |||
|
|
|||
| // Interpolated values in the properties | |||
| properties = interpolateProperties(properties); | |||
There was a problem hiding this comment.
I do not recommend changing the behavior of the readProperties step. Rather introduce a new step for interpolation if you want one. Or just leave this to a global library with @Grab for people who need it.
There was a problem hiding this comment.
Or maybe adding a new parameter telling the step to change behavior?
There was a problem hiding this comment.
Sounds better with a parameter, the impact on the code is minimal.
| <li> | ||
| <code>interpolate</code>: | ||
| Flag to indicate if the properties should be interpolated or not. | ||
| In case of error or cycling dependencies, the original properties will be returned. |
There was a problem hiding this comment.
An example would be nice here. Not all users understands what interpolated means :)
There was a problem hiding this comment.
Done! I've added an extra example with interpolation.
| @@ -39,6 +39,7 @@ | |||
| */ | |||
| public class ReadPropertiesStep extends AbstractFileOrTextStep { | |||
| private Map defaults; | |||
| private Boolean interpolate = Boolean.FALSE; | |||
There was a problem hiding this comment.
a normal, basic boolean should suffice here, it defaults to false in Java.
| * The value of <i>url</i> should be evaluated to http://localhost/resources with the interpolation on. | ||
| * @return the value of interpolated | ||
| */ | ||
| public Boolean getInterpolate() { |
There was a problem hiding this comment.
following the Java bean spec this should then be isInterpolate.
…ow Java Beans specs
…ins-ci.main:jenkins-core
| if ( properties == null) | ||
| return null; | ||
| Configuration interpolatedProp; | ||
| PrintStream logger = listener.getLogger(); |
There was a problem hiding this comment.
You need to rebase your master branch. The listener field has been removed so you are getting a compilation failure here.
It adds support for interpolated variables like
var1 = value1
var2 = ${var1}-value2
It will also expand ${sys:...} and ${env:...} keys.
As a draw back it increase the size of the pull-in because of the extra three libraries form Apache Commons.
Reference : http://commons.apache.org/proper/commons-configuration/userguide/howto_utilities.html#Utility_classes_and_Tips_and_Tricks