-
Notifications
You must be signed in to change notification settings - Fork 12
Value Mismatch #154
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Value Mismatch #154
Conversation
Bumped version to `0.4.3-SNAPSHOT`.
Expeptions are delivered via `io.grpc.Status` to client side. So even though that most likely `FailureThrowable` will be used mainly in aggregate code, the client side needs to have the generated failure classes for working with gRPC statuses.
Increment version to 0.4.4 because of strange problems with publishing. We're advancing features anyway.
|
@andrey-lavrov, please do the following:
|
To follow our Code Style convention.
Added tests for message overloading case.
| @Test | ||
| public void return_value_with_passed_values() { | ||
| final ValueMismatch mismatch = Mismatch.of(EXPECTED, FOUND, REQUESTED, VERSION); | ||
| assertTrue(mismatch.hasExpected()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please check if values are equal to expected.
|
LGTM then. |
Added tests for new methods in Value class. Added more tests to Mismatch class. Renamed parameter in Mismatch.of method
|
Fixed comments please review. On Mon, Jun 13, 2016 at 3:12 PM, AlexanderLitus notifications@github.com
|
| @Test | ||
| public void return_mismatch_object_with_double_values() { | ||
| final double value0 = 0.1; | ||
| final double value1 = 0.1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use different values as in previous methods.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's better to rename them to expected, actual, requested. And you can get rid of the existing variables by inlining them.
Renamed local test variables to make code more readable.
|
Done. |
| final int value0 = 0; | ||
| final int expected = 0; | ||
| final int value1 = 1; | ||
| final int value2 = 2; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pls rename these vars too.
Renamed local test variables to make code more readable.
|
Done |
|
LGTM. |
No description provided.