-
Notifications
You must be signed in to change notification settings - Fork 77
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
Refactor/error handling curp and client #480
Refactor/error handling curp and client #480
Conversation
e3cdf34
to
392e2ce
Compare
6b3642d
to
385c4d2
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #480 +/- ##
==========================================
- Coverage 54.23% 54.06% -0.18%
==========================================
Files 101 101
Lines 17872 17918 +46
Branches 17872 17918 +46
==========================================
- Hits 9693 9687 -6
- Misses 7599 7656 +57
+ Partials 580 575 -5
☔ View full report in Codecov by Sentry. |
f3d4c5b
to
491b55a
Compare
e6979aa
to
7a3ef92
Compare
c969695
to
ad2f80e
Compare
Signed-off-by: Phoeniix Zhao <[email protected]>
…ient Refs: xline-kv#463 Signed-off-by: Phoeniix Zhao <[email protected]>
5f05bbf
to
6cbd057
Compare
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.
LGTM!
Please add a test to cover the conversion between error types and tonic status. |
a0f5229
to
daec800
Compare
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.
LGTM
Signed-off-by: Phoeniix Zhao <[email protected]>
…tatus Signed-off-by: Phoeniix Zhao <[email protected]>
Signed-off-by: Phoeniix Zhao <[email protected]>
331f85f
to
0f3f494
Compare
Please briefly answer these questions:
Refactor the error-handling logic in Curp module and its client. FYI: #463
what problem are you trying to solve? (or if there's no problem, what's the motivation for this change?)
what changes does this pull request make?
are there any non-obvious implications of these changes? (does it break compatibility with previous versions, etc)