-
Notifications
You must be signed in to change notification settings - Fork 51
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
JENKINS-30077 - Expand build parameters #31
base: master
Are you sure you want to change the base?
Conversation
try { | ||
EnvVars vars = build.getEnvironment(listener); | ||
|
||
for (int i = 0; i < patterns.size(); i++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
patterns
can be null
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doh! Good point.
patterns.set(i, new Pattern(vars.expand(patterns.get(i).getPattern()), patterns.get(i).getType())); | ||
} | ||
externalDelete = vars.expand(externalDelete); | ||
} catch (Exception e) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What failures do we expect here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was to satisfy an error that was thrown when using EnvVars vars = build.getEnvironment(listener);
that complained there wasn't an exception catch.
EnvVars vars = build.getEnvironment(listener); | ||
|
||
for (int i = 0; i < patterns.size(); i++) { | ||
patterns.set(i, new Pattern(vars.expand(patterns.get(i).getPattern()), patterns.get(i).getType())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: Using VariableResolver
will save some object instantiations here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thx, will take a look.
Yup, that's what I figured, I just wanted to verify this was the correct location for such changes ... as it can be done at this point, or in one of the |
Given the exceptions we have to handle, this would better be done in the |
Surprising build parameters aren't supported. |
Guys, is there still an interest to finish this? |
@pjanouse Unfortunately not long after my last update I got yanked off the applicable project ... can't believe its over 2 yrs already. Feel free to take this PR, make appropriate changes and re-submit. |
I have just run into this problem. I have many files generated at each build and want to keep only the most recent. Therefore I need to use a parameter. Can this fix be finished and published? |
Initial attempt at https://issues.jenkins-ci.org/browse/JENKINS-30077
Had several options where to do
vars.expand
, open to discussion if this is not optimal location.