Handle failures in publish invocation #91
Conversation
timja
changed the title
Codecov Report
@@ Coverage Diff @@
## master #91 +/- ##
============================================
- Coverage 87.46% 85.95% -1.52%
Complexity 148 148
============================================
Files 17 17
Lines 710 726 +16
Branches 54 53 -1
============================================
+ Hits 621 624 +3
- Misses 71 84 +13
Partials 18 18
Continue to review full report at Codecov.
|
| } | ||
| } | ||
|
|
||
| private boolean publish(final StepContext context, final ChecksDetails.ChecksDetailsBuilder builder) { | ||
| @SuppressWarnings("IllegalCatch") | ||
| private boolean publish(final StepContext context, final ChecksDetails.ChecksDetailsBuilder builder) throws WithChecksPublishException { |
There was a problem hiding this comment.
is the return value still needed? would it make sense to simply throw the exception each time we want to return false? so we don't call onFailure inside the method but delegate it to the caller which I believe is safer if we want to make sure it is called only once.
There was a problem hiding this comment.
Good point! I think that the boolean return was to avoid having to create a checked exception, but it basically does the same thing now.
Given the merge conflicts and circuitous route to the current state, plus the need to rewrite it anyway...here's a new PR accomplishing the same as #83