-
Notifications
You must be signed in to change notification settings - Fork 77
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
Configuration as Code support #87
Conversation
Unit tests and Symbols
@@ -114,7 +115,8 @@ public DockerTool forNode(Node node, TaskListener log) throws IOException, Inter | |||
} | |||
} | |||
|
|||
@Extension public static class DescriptorImpl extends ToolDescriptor<DockerTool> { | |||
@Extension @Symbol("dockerTool") |
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.
It was already the symbol used, so I'm not sure adding it is confusing?
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.
Just in case the algorithm for the default name changes in the future
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.
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.
This looks great!
<workflow-cps.version>2.67</workflow-cps.version> | ||
<workflow-support.version>3.3</workflow-support.version> | ||
<pipeline-model-definition-version>1.3.8</pipeline-model-definition-version> | ||
<jcasc.version>1.41</jcasc.version> |
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 are using the plugin BOM (which is great!), the JCasC version is normally defined in it. Is it not?
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.
Not sure, a bunch of stuff isn't
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.
it seems that you can. but it's not a blocker for me
src/test/resources/org/jenkinsci/plugins/docker/commons/casc_bare.yaml
Outdated
Show resolved
Hide resolved
…are.yaml new line at end of file Co-authored-by: Adrien Lecharpentier <[email protected]>
Added tests to verify it works.
Added Symbols to make the configuration more neat looking and readable.
Updated pom to use the current JCasC version.