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.

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

change(dev tools): Add an OUTPUT_DATA_LINE_LIMIT to zcash-rpc-diff #7500

Merged
merged 1 commit into from
Sep 7, 2023

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Sep 5, 2023

Motivation

Some RPC outputs are very long, making them hard to paste into GitHub comments.

Solution

When RPC outputs are identical, only show the first 20 and last 20 lines

git diff already limits the length of its outputs. (I think?)

Review

This is a low priority developer tooling change.

Reviewer Checklist

  • Are the PR labels correct?
  • Does the code do what the ticket and PR says?
    • Does it change concurrent code, unsafe code, or consensus rules?
  • How do you know it works? Does it have tests?

@teor2345 teor2345 added P-Low ❄️ A-diagnostics Area: Diagnosing issues or monitoring performance C-testing Category: These are tests A-rpc Area: Remote Procedure Call interfaces C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG labels Sep 5, 2023
@teor2345 teor2345 requested a review from a team as a code owner September 5, 2023 23:40
@teor2345 teor2345 self-assigned this Sep 5, 2023
@teor2345 teor2345 requested review from oxarbitrage and removed request for a team September 5, 2023 23:40
@oxarbitrage
Copy link
Contributor

Can you show me where (in what RPC method) this modification was useful ?

An example output will be good enough.

@teor2345
Copy link
Contributor Author

teor2345 commented Sep 6, 2023

Can you show me where (in what RPC method) this modification was useful ?

An example output will be good enough.

There are some example outputs in this comment, click the triangles to show them:
#7450 (comment)

The original output was 2000 lines, now it's around 40 lines, so it's much easier to copy.

Copy link
Contributor

@oxarbitrage oxarbitrage left a comment

Choose a reason for hiding this comment

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

I like that the new action will only take effect when the outputs are identical.

Had a hard time trying to find an example, most of our rpc methods have at least some small difference with zcashd.

Examples in #7450 (comment) didn't worked for me (see #7450 (comment)).

However i made some dirty hacks here to check it out and everything looks good.

mergify bot added a commit that referenced this pull request Sep 7, 2023
@mergify mergify bot merged commit 2e58fa6 into main Sep 7, 2023
@mergify mergify bot deleted the limit-rpc-diff-output branch September 7, 2023 13:28
@upbqdn upbqdn mentioned this pull request Sep 22, 2023
38 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Diagnosing issues or monitoring performance A-rpc Area: Remote Procedure Call interfaces C-testing Category: These are tests C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants