Allowing a library name to be passed to the libraryResource step#49
Allowing a library name to be passed to the libraryResource step#49jmcshane wants to merge 3 commits into
Conversation
dwnusbaum
left a comment
dwnusbaum
left a comment
There was a problem hiding this comment.
Looks like a useful change to me!
| if (contents.isEmpty()) { | ||
| if (libraryName != null && contents.containsKey(libraryName)) { | ||
| return contents.get(libraryName); | ||
| } else if (contents.isEmpty() || (libraryName != null && !contents.containsKey(libraryName))) { |
There was a problem hiding this comment.
I think it would be better to have different error message for the libraryName != null && !contents.containsKey(libraryName)) condition saying the resource was found, but not in the specified library, or that has a similar message as the current one but includes the library name.
|
|
||
| @DataBoundSetter | ||
| public void setLibraryName(String libraryName) { | ||
| this.libraryName = libraryName; |
There was a problem hiding this comment.
I would use = Util.fixEmptyAndTrim(libraryName) to remove surrounding whitespace from libraryName.
|
I should be able to get around to these comments today, I’ll make sure to rebase and resolve that conflict. |
|
@dwnusbaum Could you check now? Thanks! |
| LibraryDecorator.could_not_find_any_definition_of_librari=Could not find any definition of libraries {0} | ||
| ResourceStep.library_resource_ambiguous_among_librari=Library resource {0} ambiguous among libraries {1} | ||
| ResourceStep.no_such_library_resource_could_be_found_=No such library resource {0} could be found. | ||
| ResourceStep.no_matching_resource_found_in_library=Library resource {0} not found in library {1}, but was present in other library |
There was a problem hiding this comment.
nit: Maybe reword to: "Library resource {0} not found in library {1}, although it was present in libraries {2}" and pass contents.keySet() as the third argument.
| @@ -0,0 +1,5 @@ | |||
| <div> | |||
| The shared library name to use to locate the resource. | |||
| If left blank, all loaded shared libraries will be loaded. | |||
There was a problem hiding this comment.
nit: Maybe reword to "If left blank, then this step will search for the resource in all configured shared libraries."
|
Tread carefully here. I am not convinced this is safe, which is why I did not offer such an option to begin with. |
|
Also from a usage perspective this is dubious. It would be helpful to have an explanation of what the original problem was which you are attempting to solve via this feature. |
|
Any progress with this pull request? |
|
As of #172 most of the code in this plugin has been moved to another plugin repository so this PR must be closed. If this change is still needed, please git clone https://github.com/jenkinsci/pipeline-groovy-lib-plugin
cd pipeline-groovy-lib-plugin
git checkout -b SELECT-BRANCH-NAME
git pull https://github.com/jmcshane/workflow-cps-global-lib-plugin masterresolve any merge conflicts, and file a fresh PR on the new repository. Be sure to paste a link to this old PR to enable bidirectional navigation. |
5 participants
This would allow Jenkinsfile authors to specify the library name that is included to avoid duplication across resources.