#671 rename with failure hint #970
Conversation
…robstoll#671 Update `assertions/builders/descriptiveWithFailureHint.kt`: - Add `withHelpOnFailure` and mark `withFailureHint` as deprecated - Add `withHelpOnFailureBasedOnSubject` and mark `withFailureHintBasedOnSubject` as deprecated - Add `withHelpOnFailureBasedOnDefinedSubject` and mark `withFailureHintBasedOnDefinedSubject` as deprecated Update `atrium/logic/creating/filesystem/hints/hints.kt`: - Add `withHelpOnIOExceptionFailure` and mark `withIOExceptionFailureHint` as deprecated - Add `withHelpOnFileAttributesFailure` and mark `withFileAttributesFailureHint` as deprecated Refactor all usages of `with*FailureHint` found to favor the `withHelpOn*Failure` version Update `assertions/builders/DescriptiveWithBasedOnSubjectSpec.kt`: - Refactor text referencing deprecated methods Update `atrium/logic/impl/DefaultFloatingPointAssertions.kt`: - Remove unused import for deprecated `with*FailureHint` method(s) Update `atrium/logic/impl/DefaultBigDecimalAssertions.kt`: - Remove unused import for deprecated `with*FailureHint` method(s) Update `atrium/logic/impl/DefaultPathAssertions.kt`: - Remove unused import for deprecated `with*FailureHint` method(s)
…l#671-rename-withFailureHint
| fun <T> Descriptive.DescriptionOption<Descriptive.FinalStep>.withFailureHintBasedOnDefinedSubject( | ||
| expect: Expect<T>, | ||
| failureHintFactory: (T) -> Assertion | ||
| ): Descriptive.DescriptionOption<DescriptiveAssertionWithFailureHint.FinalStep> { | ||
| return withFailureHintBasedOnSubject(expect) { | ||
| return withHelpOnFailureBasedOnSubject(expect) { |
There was a problem hiding this comment.
I would delegate to withHelpOnFailureBasedOnDefinedSubject but also fine like this
There was a problem hiding this comment.
Hey @robstoll! I can change that if you would like for me to do so - I was just flipping any old with*failureHint method names to the new equivalent on all of the usages I could find to make sure I didn't introduce any unexpected changes. I'll adjust this on your signal.
|
@stevenlmcgraw looks good to me. Only a small thing which you can improve in case you do other things in addition (I did not have a look if you replaced everything). If you have done everything, then there is no need for the improvement and we can merge your first PR :) |
|
Howdy @robstoll ! I used the same approach you suggested to another individual in the comments for this issue, i.e., I searched the whole project in IntelliJ with the |
|
@stevenlmcgraw sounds good, let's wait to see if the build is green as well. |
Codecov Report
@@ Coverage Diff @@
## master #970 +/- ##
============================================
- Coverage 90.89% 90.86% -0.03%
============================================
Files 433 433
Lines 4336 4359 +23
Branches 217 219 +2
============================================
+ Hits 3941 3961 +20
- Misses 345 349 +4
+ Partials 50 49 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
@stevenlmcgraw don't worry about the coverage. It's perfect if you write a test for it but it's also OK if we keep it like that |
|
@robstoll Gotcha - I don't think I would be able to get to working on some unit tests until later this week. I don't mind taking a crack at it, but I don't want to hold you up if you are looking to get this merged as quickly as possible. |
|
@stevenlmcgraw I am merging it already but a test would still be welcome. You can create a second PR with it. In this sense, thanks for your first contribution to Atrium 👍 and don't forget to ⭐ the repo if you liked it 🙂 |
|
Thanks @robstoll ! I starred it. I'll try to set aside some time outside of work to do the tests. I'll be looking at some more issues here in Atrium as well - catch you on the next one! |
|
@stevenlmcgraw ping 😉 we could use your help with other issues. Let me know in case you need help |
Draft pull request for #671
I confirm that I have read the Contributor Agreements v1.0, agree to be bound on them and confirm that my contribution is compliant.