-
Notifications
You must be signed in to change notification settings - Fork 373
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
fix(gnodev): skip failing tx instead of raising an error #1456
Conversation
Signed-off-by: gfanton <[email protected]>
Signed-off-by: gfanton <[email protected]>
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.
Makes sense to me 👍
Signed-off-by: gfanton <[email protected]>
Signed-off-by: gfanton <[email protected]>
0ea2a0f
to
c023dd9
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1456 +/- ##
==========================================
+ Coverage 56.08% 56.10% +0.02%
==========================================
Files 432 432
Lines 66000 66727 +727
==========================================
+ Hits 37016 37440 +424
- Misses 26095 26366 +271
- Partials 2889 2921 +32
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
The changes look good so approving.
I'm wondering though: should an end-user not clearly be able to see any error publishing the specific packages they're publishing?
I get what Hariom is saying in the issue, however if I'm working on a realm and I introduce a bug, I would want gnodev
to prominently and clearly tell me what the generated error/panic while publishing the package is.
Not sure if my expectations are realistic or not, but I expect output something like: Case 1: When $ ~/De/w/gno/examples ❯ gnodev gno.land/r/demo/helloworld
Node | Listener: tcp://0.0.0.0:36657
[...]
Node | Loading packages...
Node | Failed to load `gno.land/p/demo/avl`...
- cannot use 0 (untyped int constant) as *Tree value in return statement
Node | Successfully loaded `gno.land/r/demo/helloworld`...
Node | [DONE - with 1 error] In this case, I can continue working on Case 1: When $ ~/De/w/gno/examples ❯ gnodev gno.land/r/demo/helloworld
Node | Listener: tcp://0.0.0.0:36657
[...]
Node | Loading packages...
Node | Failed to load `gno.land/p/demo/avl`...
- cannot use 0 (untyped int constant) as *Tree value in return statement
Node | Failed to load `gno.land/r/demo/helloworld`...
- Missing package gno.land/p/demo/avl
Node | [DONE - with 2 errors] In this case, since i imports We should also add |
Co-authored-by: Hariom Verma <[email protected]>
@harry-hov @thehowl it should be possible, I have an idea on how we can implement this. but I will do this in another PR |
Co-authored-by: Hariom Verma <[email protected]>
fix #1447
Skip failing TX instead of raising an error.
Also adjust/add some comments in dev node package for better understanding and readability
cc @harry-hov
Contributors' checklist...
BREAKING CHANGE: xxx
message was included in the description