Validate release is a String during configuration#2040
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #2040 +/- ##
==========================================
+ Coverage 77.37% 83.15% +5.78%
==========================================
Files 110 119 +9
Lines 4954 5645 +691
==========================================
+ Hits 3833 4694 +861
+ Misses 1121 951 -170
☔ View full report in Codecov by Sentry. |
| end | ||
| end | ||
|
|
||
| def check_instance!(name, value, klass) |
There was a problem hiding this comment.
We already have a helper to check argument types. Maybe we can update that method so it accepts 2 expected types?
There was a problem hiding this comment.
hmm I don't even think we need the nil check because no one should be setting release explicity to nil, I'll change the release_detector assignment above to an instance variable.
edit: nvm, breaks some other tests, will extend
There was a problem hiding this comment.
It may be easier for users because they can just set this and don't need to worry about if the env is present.
config.release = ENV["MY_RELEASE"]If we raise in that case, users need to write
config.release = ENV["MY_RELEASE"] if ENV["MY_RELEASE"] AFTER seeing at least one error.
61ff2ad to
3a7acbb
Compare
3a7acbb to
56ee1a1
Compare
st0012
left a comment
There was a problem hiding this comment.
Looks good. I will try a small refactor after this is merged.
BTW, is it possible that we can work with codecov developers to improve the coverage accuracy in this repo? These false warnings are pretty annoying 😕
closes #2004