Skip to content
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

Revert unix command to command to prevend breaking changes #127

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Roemer
Copy link
Collaborator

@Roemer Roemer commented Sep 15, 2023

Revert an incompatible change from #118

@Roemer Roemer force-pushed the feature/Revert-Incompatible-Command branch from 2cc4db6 to ab8dd92 Compare September 15, 2023 10:02
Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

must also be renamed to retain compatibility in $JENKINS_HOME/config.xml. (You can use @LocalData to test this sort of thing by the way.) configuration-as-code users on the other hand will need renamed to match the constructor parameter name.

As an aside, similar to https://www.jenkins.io/doc/developer/plugin-development/pipeline-integration/#constructor-vs-setters it is recommended to limit @DataBoundConstructor parameters to those fields which are obligatory (no sensible default could be provided), and use @DataBoundSetter for everything else. If you do this right, config.xml can avoid serializing fields unmodified from their default, and JCasC users will need only set interesting fields (and exporting JCasC YAML will display only those fields which were really edited).

<f:entry title="Unix Command" field="unixCommand">
<f:textarea value="${dockerSwarmAgentTemplate.unixCommand}" default="sh&#10;-cx&#10;curl --connect-timeout 20 --max-time 60 -o agent.jar $DOCKER_SWARM_PLUGIN_JENKINS_AGENT_JAR_URL &amp;&amp; java -classpath agent.jar hudson.remoting.jnlp.Main -headless -url $DOCKER_SWARM_PLUGIN_JENKINS_URL -noreconnect -workDir /tmp $DOCKER_SWARM_PLUGIN_JENKINS_AGENT_SECRET $DOCKER_SWARM_PLUGIN_JENKINS_AGENT_NAME"/>
<f:entry title="Unix Command" field="command">
<f:textarea value="${dockerSwarmAgentTemplate.command}" default="sh&#10;-cx&#10;curl --connect-timeout 20 --max-time 60 -o agent.jar $DOCKER_SWARM_PLUGIN_JENKINS_AGENT_JAR_URL &amp;&amp; java -classpath agent.jar hudson.remoting.jnlp.Main -headless -url $DOCKER_SWARM_PLUGIN_JENKINS_URL -noreconnect -workDir /tmp $DOCKER_SWARM_PLUGIN_JENKINS_AGENT_SECRET $DOCKER_SWARM_PLUGIN_JENKINS_AGENT_NAME"/>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way just

Suggested change
<f:textarea value="${dockerSwarmAgentTemplate.command}" default="sh&#10;-cx&#10;curl --connect-timeout 20 --max-time 60 -o agent.jar $DOCKER_SWARM_PLUGIN_JENKINS_AGENT_JAR_URL &amp;&amp; java -classpath agent.jar hudson.remoting.jnlp.Main -headless -url $DOCKER_SWARM_PLUGIN_JENKINS_URL -noreconnect -workDir /tmp $DOCKER_SWARM_PLUGIN_JENKINS_AGENT_SECRET $DOCKER_SWARM_PLUGIN_JENKINS_AGENT_NAME"/>
<f:textarea default="sh&#10;-cx&#10;curl --connect-timeout 20 --max-time 60 -o agent.jar $DOCKER_SWARM_PLUGIN_JENKINS_AGENT_JAR_URL &amp;&amp; java -classpath agent.jar hudson.remoting.jnlp.Main -headless -url $DOCKER_SWARM_PLUGIN_JENKINS_URL -noreconnect -workDir /tmp $DOCKER_SWARM_PLUGIN_JENKINS_AGENT_SECRET $DOCKER_SWARM_PLUGIN_JENKINS_AGENT_NAME"/>

should work (same for other properties): the point of the field attr on f:entry is that it takes care of the plumbing needed for config form round-trips so long as you are consistent in your naming.

@jglick
Copy link
Member

jglick commented Feb 26, 2024

Stalled?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants