-
-
Notifications
You must be signed in to change notification settings - Fork 52
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
Release 3.0.0 #166
Release 3.0.0 #166
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #166 +/- ##
=======================================
Coverage 95.34% 95.34%
=======================================
Files 2 2
Lines 172 172
=======================================
Hits 164 164
Misses 8 8 ☔ View full report in Codecov by Sentry. |
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'd like to flip the default for facterdb_string_keys
, but perhaps it's too soon for 3.0.0. Should we issue a deprecation warning in 3.0.0?
An alternative: finish #132 first and provide a proper API to override facts. So you can write:
require 'spec_helper'
describe_on_supported_os 'myclass' do
it { is_expected.to compile.with_all_deps }
context 'with selinux' do
override_facts(selinux: true)
# or
override_facts_merged(os: { selinux: { enabled: true }})
it { is_expected.to compile.with_all_deps }
end
end
Then it just does the right thing for you with less boilerplate. Only then issue the deprecation.
Though you would still need to deal with the selection on certain OS facts even with the improved DSL.
I also agree that it's too early to flip to strings. Releasing the current changes fixes some bugs, but if the rollout requires the change to strings it's just a too big change. A deprecation warning is a good idea but I've no idea how/where to properly implement this. |
4c2d293
to
d37fafa
Compare
I added a deprecation notice to the CHANGELOG.md |
Waiting a bit for @ekohl in case he want's other breaking changes.
CC: @jordanbreen28