-
Notifications
You must be signed in to change notification settings - Fork 98
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
fedekunze/1131 show only err msgs on notifications #1539
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #1539 +/- ##
===========================================
+ Coverage 96.4% 96.41% +<.01%
===========================================
Files 97 97
Lines 1810 1811 +1
Branches 86 86
===========================================
+ Hits 1745 1746 +1
Misses 55 55
Partials 10 10
|
@@ -82,14 +82,14 @@ function sleep(ms) { | |||
return new Promise(resolve => setTimeout(resolve, ms)) | |||
} | |||
|
|||
function handleCrash(error) { | |||
function handleCrash(err) { |
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.
going from a non abbreviated word to an abbreviation feels weird.
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.
error
is a reserved word in some programming languages. Also it makes it easier to search retrieved error responses
Do you think this is an advantage? I think the main problem was showing the correct errors from the LCD and not sth like |
The correct errors from the LCD are fixed on the SDK. The missing part was verifying that we didn't append any additional message on top of them |
Closes #1131
Description:
❤️ Thank you!
CHANGELOG.md
with issue # and GitHub usernameFiles changed
in the github PR explorer