-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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-58767] Persist yaml fragment in a single string #573
Conversation
|
||
private List<String> yamls = new ArrayList<>(); | ||
/** | ||
* List of yaml fragments used for transient pod templates. Never persisted |
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.
Could this field just be deleted, and any call to setYamls
with a list of size > 1 would just set yaml
to something separated by "\n----\n"
or whatever? Would be easier to follow the logic and be sure that you are not missing some complication.
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.
hum, I guess it would increase complexity of getYaml
, not sure I want it
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.
Problem I see is that would end up being an invalid yaml fragment to parse into a pod template
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.
As you see fit. I do not really understand what the list of YAMLs is for to begin with.
@@ -96,6 +101,19 @@ public void upgradeFrom_1_15_9() { | |||
assertNull(template._getYamls()); | |||
} | |||
|
|||
@Test | |||
@LocalData | |||
public void upgradeFrom_1_15_9_invalid() { |
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.
Probably overkill, but OK.
…ct yaml is not null Amends jenkinsci#573
JENKINS-58767