stschulte / puppet-rpmkey

Custom puppet type for managing GPG Keys inside rpm
The Unlicense
10 stars 9 forks source link

autorequire a local source #4

Closed duritong closed 9 years ago

duritong commented 9 years ago

if the local key to import is also managed by puppet, this adds an automatic require dependency on that file.

stschulte commented 9 years ago

@duritong Thanks for the suggestions and the pull request. I added a few inline comments. Please let me know what you think and if you are willing to make the changes yourself. Otherwise I am happy to take it from here.

duritong commented 9 years ago

Except of the additional test cases I'm fine with your suggestions. Once we can come up with testcases for urls and nil, that actually make sense, I can add them and do the other changes.

duritong commented 9 years ago

I changed the spec, everything else should be fine now?

stschulte commented 9 years ago

@duritong I'd still like to have a test case that the block passed to the autorequire method does not blow up when source is empty. I mean I know that it works as it is right now, but suppose someone would like to do some sanitize first and

self[:source] if self[:source] =~ /^\//

becomes

# can fail with undefined method `sub' for nil:NilClass
self[:source] if self[:source].sub('a','b') =~ /^\//

this would not cause any tests to fail right now.

duritong commented 9 years ago

like this?

stschulte commented 9 years ago

@duritong thanks, that looks good. I've merged your changes and prepared a new release for the forge. Thanks again!

duritong commented 9 years ago

:-)