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

Allow :value in addition to :default for setting attribute values #3759

Merged
merged 4 commits into from Jan 28, 2019

Conversation

Projects
None yet
3 participants
@clintoncwolfe
Copy link
Contributor

clintoncwolfe commented Jan 28, 2019

Closes #3756

This PR implements #3756, by using the word 'value' instead of 'default' to set attribute values.

  • A deprecation group is added, action 'ignore'.
  • 'default' is still accepted, but internally is immediately converted to 'value'.
  • unit tests are added to check that the 'default' word still works
  • All test fixtures and examples are updated to use 'value'
  • The reference docs are updated.

clintoncwolfe added some commits Jan 25, 2019

Add deprecation hook for attribute 'default' option
Signed-off-by: Clinton Wolfe <clintoncwolfe@gmail.com>
Update docs
Signed-off-by: Clinton Wolfe <clintoncwolfe@gmail.com>
Update value/default usage in test files
Signed-off-by: Clinton Wolfe <clintoncwolfe@gmail.com>
Test and fix case in which both :default and :value are provided
Signed-off-by: Clinton Wolfe <clintoncwolfe@gmail.com>

@clintoncwolfe clintoncwolfe changed the title Use :value instead of :default for setting attribute values Allow :value in addition to :default for setting attribute values Jan 28, 2019

@tas50

tas50 approved these changes Jan 28, 2019

Copy link
Contributor

tas50 left a comment

Seems pretty straight forward and killing the name "default" which has a very specific meaning in Chef is great

attribute = Inspec::Attribute.new('test_attribute', required: true)
attribute.value = 'test_value'
attribute.value.must_equal 'test_value'
end

it 'returns an error if no value is set' do
# Assigning the cli_command is needed because in check mode, we don't error

This comment has been minimized.

@jerryaldrichiii

jerryaldrichiii Jan 28, 2019

Contributor

Odd...I feel like we should error...

This comment has been minimized.

@clintoncwolfe

clintoncwolfe Jan 28, 2019

Author Contributor

In check mode (inspec check) it is assumed that we won't have a --attrs file passed in (we might not even support the option).

So, for better or worse, don't treat a missing-but-required attribute as an error in check mode. We could change that policy, but remember things like A2 run check.

@clintoncwolfe clintoncwolfe merged commit ce15ee9 into master Jan 28, 2019

4 checks passed

DCO This commit has a DCO Signed-off-by
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
expeditor/config-validation Validated your Expeditor config file
Details

@clintoncwolfe clintoncwolfe deleted the cw/attrs-deprecate-default branch Jan 28, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment