show threshold in failure message of BeNumericallyMatcher #293
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
grosser
force-pushed
the
grosser/threshold
branch
from
d4a98ce
to
dcfe84c
Compare
williammartin
requested changes
Aug 6, 2018
matchers/be_numerically_matcher.go
Outdated
| if !positive { | ||
| formatted = "not " + formatted | ||
| } | ||
| formatted = format.Message(actual, formatted, matcher.CompareTo[0]) |
There was a problem hiding this comment.
Can we just return here in place of setting the formatted variable and then returning?
matchers/be_numerically_matcher.go
Outdated
| } | ||
|
|
||
| func (matcher *BeNumericallyMatcher) FormatFailureMessage(actual interface{}, positive bool) (message string) { | ||
| var formatted string |
There was a problem hiding this comment.
Following on from the comment below, maybe we could just rename this to message, since it's the thing that will be formatted, instead of the thing that is formatted.
matchers/be_numerically_matcher.go
Outdated
| return matcher.FormatFailureMessage(actual, false) | ||
| } | ||
|
|
||
| func (matcher *BeNumericallyMatcher) FormatFailureMessage(actual interface{}, positive bool) (message string) { |
There was a problem hiding this comment.
Could we maybe change positive here to negated, and flip the boolean expressions to be consistent with the Method names. Also, can we make this private formatFailureMessage?
grosser
force-pushed
the
grosser/threshold
branch
from
dcfe84c
to
a850844
Compare
|
updated @williammartin |
williammartin
approved these changes
Aug 7, 2018
|
LGTM thanks. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
solves onsi/ginkgo#503 (comment)
@williammartin