-
Notifications
You must be signed in to change notification settings - Fork 681
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
add shadow resource for /etc/shadow #471
Conversation
|
||
def initialize(path = nil, opts = nil) | ||
opts ||= {} | ||
@path = path || '/etc/shadow' |
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.
nitpicking, but how about
def initialize(path = '/etc/shadow', opts = {})
@arlimus: great resource. Went a little bonkers with non-important code style comments ;) LGTM 👍 |
@@ -0,0 +1,130 @@ | |||
# encoding: utf-8 | |||
# copyright: 2015, Chef Software Inc. |
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.
2016 🎉
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.
ups :)
f5f187c
to
2ee967e
Compare
|
||
def initialize(path = '/etc/shadow', opts = nil) | ||
opts ||= {} | ||
@path = path || '/etc/shadow' |
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.
This is redundant now, isn't it?
@path = path
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.
Almost: what if shadow(nil)
. It will not happen intentionally, but may happen by accident. I'm not sure yet how we want to handle this case though. Throwing a ruby fail seems too harsh, maybe improve this in the next iteration. It is true for a lot of resources...
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.
Ok, another edge case I didn't notice. Thanks.
My original comment then doesn't make too much sense then (i.e. the case against path=nil
)...
That looks really good. I like to see filter everywhere now. |
Thanks @arlimus |
add shadow resource for /etc/shadow
No description provided.