Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Print state root after transaction completion when --jsontrace is specified #212

Merged
merged 2 commits into from
Dec 13, 2017
Merged

Print state root after transaction completion when --jsontrace is specified #212

merged 2 commits into from
Dec 13, 2017

Conversation

jwasinger
Copy link
Contributor

@jwasinger jwasinger commented Oct 4, 2017 •
edited
Loading

@holgerd77
Copy link
Member

There is a t.comment function I also recently discovered in tape like here for custom test output to the console, can we try to consistently use this instead of console.log (maybe also change the other occurrences through this PR)?

This gives a more consistent output stream e.g. for channeling the output to another stdout or using custom stream reporters for tape (https://github.com/substack/tape#var-stream--testcreatestreamopts).

@holgerd77
Copy link
Member

(one more background common on this: experimented with stream reporters couple of weeks ago, think it would be nice to be a bit more flexible on test output, e.g. to leave all the success cases output when most of the tests pass).

@jwasinger
Copy link
Contributor Author

Cool find!

@axic
Copy link
Member

axic commented Dec 10, 2017

Is something blocking the merge of this?

@jwasinger jwasinger force-pushed the stateroot-in-jsontrace branch from 382ddc1 to cc997c8 Compare December 12, 2017 05:00
@jwasinger
Copy link
Contributor Author

Nope. please review and merge if you get a chance.

@jwasinger jwasinger requested review from axic and holgerd77 December 12, 2017 06:54
axic
axic previously approved these changes Dec 12, 2017
View reviewed changes
Copy link
Member

@axic axic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compared to the tests issue this looks ok.

@jwasinger jwasinger force-pushed the stateroot-in-jsontrace branch from cc997c8 to 7637d10 Compare December 12, 2017 22:36
@holgerd77
Copy link
Member

@jwasinger Can you use the t.comment function for this as we discussed above?

@jwasinger jwasinger force-pushed the stateroot-in-jsontrace branch from 71adf6a to a67449e Compare December 13, 2017 20:47
holgerd77
holgerd77 approved these changes Dec 13, 2017
View reviewed changes
Copy link
Member

@holgerd77 holgerd77 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested this locally, looks good.

@holgerd77 holgerd77 merged commit 9f8f66e into master Dec 13, 2017
@holgerd77 holgerd77 deleted the stateroot-in-jsontrace branch December 13, 2017 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@holgerd77 holgerd77 holgerd77 approved these changes

@axic axic axic left review comments

Assignees
No one assigned
Labels
package: vm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jwasinger @holgerd77 @axic @evertonfraga