-
Notifications
You must be signed in to change notification settings - Fork 568
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
generate plugin_parent_dir #102
Conversation
@@ -4,7 +4,7 @@ | |||
define jenkins::plugin($version=0) { | |||
$plugin = "${name}.hpi" | |||
$plugin_dir = '/var/lib/jenkins/plugins' | |||
$plugin_parent_dir = '/var/lib/jenkins' | |||
$plugin_parent_dir = inline_template('<%= @plugin_dir.split(\'/\')[0..-2].join(\'/\') %>') |
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.
Puppet DSL evaluating Erb, evaluating Ruby, there's got to be a better way!
There should also be some tests ;)
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.
I think dirname is better, but can't use it without excluding PE (stdlib 4.x)
There is a test, already in place that covers this I think. Is there additional tests you'd like?
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.
Hrm, I suppose this is covered by spec/defines/jenkins_plugin_spec.rb
but only incidentally since that spec makes sure the file
resource gets specified.
I'm really not sure what kind of additional testing should be done here, but this line makes me nervous :P
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.
Incidentally, totally agree, but not sure how else to test it since you can't test a variable directly.
Honestly I'd be fine just closing this since the path is hard coded and closing the original issue as well as "Won't fix". I think this goes back to some of the requests of being able to set JENKINS_HOME which would end up being $plugin_parent_dir.
Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests |
Closing this since it's merged into |
closes #89