add a default template path for when we can't find a pipeline_template#195
add a default template path for when we can't find a pipeline_template#195AboorvaDevarajan wants to merge 1 commit intojenkinsci:masterfrom
Conversation
AboorvaDevarajan
changed the title
justinharringa
left a comment
justinharringa
left a comment
There was a problem hiding this comment.
Hey there! Thank you so much for your contribution! I didn't get a lot of time to look at this but went ahead and sent some questions/suggestions. I'd definitely like to hear your point of view.
Hope all is well!
| <f:entry title="${%Config File Path}" field="scriptPath"> | ||
| <f:textbox default=".yourconfig.yml"/> | ||
| </f:entry> | ||
| <f:entry title="${%Pipeline Template Path}" field="pipelinePath"> |
There was a problem hiding this comment.
I wonder if Default Pipeline Template Path might be a better descriptor title? Perhaps renaming the field to defaultPipelinePath if that makes sense?
| @@ -0,0 +1,3 @@ | |||
| <div> | |||
| Relative default location of pipeline_template. | |||
There was a problem hiding this comment.
It might be good to describe the example. It doesn't have to be this suggestion but maybe something like this:
| Relative default location of pipeline_template. | |
| Relative default location of <code>pipeline_template</code>. This is useful when you have some repositories in an organization where you do not want to customize any values for the template or you cannot add a Config File to the repository. |
| @@ -0,0 +1,3 @@ | |||
| <div> | |||
| Relative default location of pipeline_template. | |||
There was a problem hiding this comment.
Same suggestion as above 😄
| Relative default location of pipeline_template. | |
| Relative default location of <code>pipeline_template</code>. This is useful when you have some repositories in an organization where you do not want to customize any values for the template or you cannot add a Config File to the repository. |
| String pipelineTemplateNotFound = | ||
| String.format("Could not find a value for %s in %s", PIPELINE_TEMPLATE, scriptPath); | ||
| throw new AbortException(pipelineTemplateNotFound); | ||
| jenkinsfilePathString = pipelinePath; |
There was a problem hiding this comment.
I wonder if adding a log entry here might be useful and perhaps providing a different AbortException at https://github.com/jenkinsci/config-driven-pipeline-plugin/pull/195/files#diff-cb4b4fb0cb84e1621bd3517151cb5d3308b9555208d5aed521263075d12603cbR195
What I'm thinking is that it might be hard to tell which of these code branches the plugin has taken and whether I have a misconfiguration in the plugin or if my repo simply isn't correct. Basically, seems like it'd be nice to help the user and admin understand.
Perhaps we could log whether the Config File was found, whether pipeline_template was found, and/or if we're taking the Default Pipeline Template Path
| public class ConfigDrivenWorkflowBranchProjectFactory extends AbstractWorkflowBranchProjectFactory { | ||
| // TODO: Make this a parameter that users can adjust to their liking | ||
| public static final String USER_DEFINITION_PATH = ".yourconfig.yml"; | ||
| public static final String USER_DEFINITION_PIPELINE_PATH = "Jenkinsfile"; |
There was a problem hiding this comment.
Depending on whether we go with the name change for the field, it might be good to name this closer to what it is. 😄
| public static final String USER_DEFINITION_PIPELINE_PATH = "Jenkinsfile"; | |
| public static final String DEFAULT_PIPELINE_TEMPLATE_PATH = "Jenkinsfile"; |
|
This is something that would be very useful to me, as we are migrating off of the paid Cloudbees version of Jenkins and this is the behavior of their organization folder, where a jenkins file can be specified as part of the configuration instead of on a per-project basis. This allows us to use the same configuration file for CI, CD, and Testing but have seperate Jenkins files for each. |
|
@afalko could we get a review of this PR, and if not acceptable, let us know what else needs to be done to get it merged? |
3 participants
Added a configuration option to configure the default path for
pipeline_templateinstead of failing it when thepipeline_templatedoesn't exist.Related to Issue: #25