Bug 997763 #438
Bug 997763 #438
Conversation
| end | ||
|
|
||
| def jenkins_client_cartridges | ||
| filter_cartridges { |c| (c.tags || []).include?('ci_builder') && c.name =~ /jenkins.*/ }.sort |
There was a problem hiding this comment.
You need to raise if the cartridge isn't there
raise JenkinsNotInstalledOnServer, "There is no installed cartridge that exposes Jenkins"
and a separate one for jenkins. Also, i would recommend
c.name.downcase.include? "jenkins"
rather than the regex
There was a problem hiding this comment.
Needs a test case exposing the failure case.
There was a problem hiding this comment.
Cool. What about
c.name =~ /jenkins./im
(regex ignoring case). Also note this regex will match if name *starts with 'jenkins' (different than 'include?').
/me <3 regex :)
There was a problem hiding this comment.
If you're going to do starts_with, use /\Ajenkins/i. We should never have multiline names, and \A is clearer than the greedy .*
|
@smarterclayton review again please |
|
Reviewed, [merge] |
|
Online Merge Results: SUCCESS (https://ci.dev.openshift.redhat.com/jenkins/job/test_pull_requests/4010/) (Image: devenv_3681) |
|
[Test]ing while waiting on the merge queue |
|
Origin Test Results: Waiting for stable build of 'origin_ami' |
|
Online Test Results: SUCCESS (https://ci.dev.openshift.redhat.com/jenkins/job/test_pull_requests/4010/) |
|
Evaluated for online up to 733e20d |
Merged by openshift-bot
No description provided.