Skip to content
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

Avoid constant warnings in windows/filesystem plugin #1360

Merged
merged 2 commits into from
Jun 4, 2019
Merged

Conversation

tas50
Copy link
Contributor

@tas50 tas50 commented May 3, 2019

This is very noisy in DK specs.

Signed-off-by: Tim Smith tsmith@chef.io

Copy link

@zenspider zenspider left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plain feedback. I don't feel like I'm in a position to request changes on this one. Anything that cleans up the warnings is better than the current situation of the rspec output.

@@ -24,7 +24,7 @@
# @see https://docs.microsoft.com/en-us/windows/desktop/SecProv/getconversionstatus-win32-encryptablevolume#parameters
#
CONVERSION_STATUS = %w{FullyDecrypted FullyEncrypted EncryptionInProgress
DecryptionInProgress EncryptionPaused DecryptionPaused}.freeze
DecryptionInProgress EncryptionPaused DecryptionPaused}.freeze unless defined?(CONVERSION_STATUS)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. I feel like this is a bandaid that hides a problem in plugin.rb (NamedPlugin.plugin is where this seems to be coming from).
  2. You can change that to ||= instead of unless defined?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated this to improve our bandaid to use ||= everywhere

@tas50 tas50 requested a review from a team as a code owner June 3, 2019 20:42
tas50 added 2 commits June 3, 2019 13:56
This is very noisy in DK specs.

Signed-off-by: Tim Smith <tsmith@chef.io>
This is a lot easier to read.

Signed-off-by: Tim Smith <tsmith@chef.io>
@tas50 tas50 merged commit 35b0d6f into master Jun 4, 2019
@chef-ci chef-ci deleted the CONVERSION_STATUS branch June 4, 2019 00:06
@lock
Copy link

lock bot commented Aug 3, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants