Add Constant Scalar #637
Add Constant Scalar #637
Conversation
| MatcherAssert.assertThat( | ||
| "Can't return given value", | ||
| new Constant<>("Hello World").value(), | ||
| Matchers.endsWith("World") |
There was a problem hiding this comment.
@Aliceice Why not Matchers.equalTo("Hello World") here? endsWith does not guarantee that Constant doesn't change the encapsulated String somehow.
There was a problem hiding this comment.
The problem here is qulice. It does not allow to use the same String twice in a class. So either I define a variable beforehand and use this in both construction and assertion, or do it like this.
What do you suggest?
There was a problem hiding this comment.
@Aliceice Right, I forgot how strict Qulice is... then I would say declare a variable beforehand :)
| * | ||
| * <p>Example: | ||
| * <pre> | ||
| * final Scalar<String> constant = new Constant("Pre-Computed Value"); |
There was a problem hiding this comment.
@Aliceice If I'm not mistaken, here you should have new Constant<>("Pre-Computed Value") (missing the <>). Also, I think a ; is missing after >.
|
@Aliceice I also saw that you opened a ticket in I'm not sure what exactly is not supported by qulice; the dash ( |
|
@amihaiemil Qulice is not recognizing the dash |
Codecov Report
@@ Coverage Diff @@
## master #637 +/- ##
==========================================
+ Coverage 69.96% 70% +0.03%
- Complexity 1001 1003 +2
==========================================
Files 213 214 +1
Lines 3300 3304 +4
Branches 192 192
==========================================
+ Hits 2309 2313 +4
Misses 944 944
Partials 47 47
Continue to review full report at Codecov.
|
|
@amihaiemil I updated the PR according to your suggestions. Could you please have a look again? |
|
@Aliceice looks good! |
|
@rultor good to merge |
@amihaiemil Thanks for your request. @yegor256 Please confirm this. |
|
@yegor256 ping |
|
@rultor merge |
|
Order was successfully finished: +15 points just awarded to @amihaiemil/z, total is +615 |
|
The job #637 is now out of scope |
|
@Aliceice PR is now merged. Don't forget to ask the initial ticket's reporter to close it. |
ghost
deleted the
ghost
mentioned this pull request
As per #609