chore: remove deprecated method#95
Conversation
Codecov Report
@@ Coverage Diff @@
## master #95 +/- ##
=========================================
Coverage 77.32% 77.32%
Complexity 1108 1108
=========================================
Files 73 73
Lines 5915 5915
Branches 645 645
=========================================
Hits 4574 4574
Misses 1014 1014
Partials 327 327Continue to review full report at Codecov.
|
elharo
left a comment
There was a problem hiding this comment.
Per semver removing a deprecated method requires a major version bump because it's backwards incompatible. I suspect that should be done in this PR.
| } | ||
|
|
||
| @Test | ||
| @Test(expected = NullPointerException.class) |
There was a problem hiding this comment.
Google best practice is not to use expected exceptions. As of JUnit 4.13 you can use assertThrows instead.
Codecov Report
@@ Coverage Diff @@
## master #95 +/- ##
============================================
- Coverage 77.32% 77.26% -0.06%
Complexity 1108 1108
============================================
Files 73 73
Lines 5915 5904 -11
Branches 645 635 -10
============================================
- Hits 4574 4562 -12
+ Misses 1014 1011 -3
- Partials 327 331 +4
Continue to review full report at Codecov.
|
elharo
left a comment
There was a problem hiding this comment.
We shouldn't change the public API without a major version bump.
| try { | ||
| new Option(null, VALUE) {}; | ||
| Assert.fail(); | ||
| } catch (NullPointerException ex) { |
There was a problem hiding this comment.
ex --> expected in this pattern, to avoid problems with static code analysis
| new Option(null, VALUE) {}; | ||
| Assert.fail(); | ||
| } catch (NullPointerException ex) { | ||
| assertNull(ex.getMessage()); |
There was a problem hiding this comment.
I suppose this is current behavior, but there should be a message here and I'd prefer not to lock in its nonexistence with this assert.
Fixes #68