-
Notifications
You must be signed in to change notification settings - Fork 682
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
Fix and document registry issues #1635
Conversation
Signed-off-by: Christoph Hartmann <[email protected]>
…handled Signed-off-by: Christoph Hartmann <[email protected]>
Signed-off-by: Christoph Hartmann <[email protected]>
Signed-off-by: Christoph Hartmann <[email protected]>
1b53a31
to
266241f
Compare
lib/resources/registry_key.rb
Outdated
@@ -66,7 +66,10 @@ def initialize(name, reg_key = nil) | |||
# generate registry_key if we do not have a regular expression | |||
@options[:path] = @options[:hive] | |||
# add optional key path | |||
@options[:path] += '\\' + @options[:key] if @options[:key] | |||
if @options[:key] |
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.
Can we break out the building of the path to a private helper method in this class and add some tests to it? There's enough conditional logic in building the path now that I think it's warranted.
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 agree with you @adamleff This would increase the quality!
Broke out some of the conditional logic in the `#initialize` method into helper methods and added tests. Signed-off-by: Adam Leff <[email protected]>
Thank you @adamleff for improving my PR. |
Powershell 3 has issues with some characters in strings when used in combination with
ConvertTo-Json
:By using
ConvertTo-Json -Compress
, it just works:registry_key
behavior (Unable to get .NET Version) #1131registry_key
#1268