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

fix(dev): Make zcash-rpc-diff always check Zebra then zcashd #5822

Merged
merged 1 commit into from
Dec 9, 2022

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Dec 8, 2022

Motivation

Sometimes they were in the opposite order in the script, which is confusing when reviewing diffs.

Review

This is a minor usability fix for a developer testing tool.

I used it for the 3 diffs in:
#5808 (comment)

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 ❄️ 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 Dec 8, 2022
@teor2345 teor2345 requested a review from a team as a code owner December 8, 2022 06:46
@teor2345 teor2345 self-assigned this Dec 8, 2022
@teor2345 teor2345 requested review from arya2 and removed request for a team December 8, 2022 06:46
@teor2345 teor2345 changed the title fix(dev): Make zcash-rpc-diff always do Zebra then zcashd fix(dev): Make zcash-rpc-diff always check Zebra then zcashd Dec 8, 2022
@github-actions github-actions bot added the C-bug Category: This is a bug label Dec 8, 2022
@codecov
Copy link

codecov bot commented Dec 8, 2022

Codecov Report

Merging #5822 (35b48f3) into main (2041fda) will increase coverage by 0.04%.
The diff coverage is n/a.

❗ Current head 35b48f3 differs from pull request most recent head ef7aa5a. Consider uploading reports for the commit ef7aa5a to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5822      +/-   ##
==========================================
+ Coverage   78.69%   78.73%   +0.04%     
==========================================
  Files         308      308              
  Lines       38781    38781              
==========================================
+ Hits        30517    30535      +18     
+ Misses       8264     8246      -18     

@arya2 arya2 changed the base branch from main to getmininginfo-use-u64 December 9, 2022 00:44
@arya2 arya2 changed the base branch from getmininginfo-use-u64 to main December 9, 2022 00:44
arya2
arya2 previously approved these changes Dec 9, 2022
@arya2
Copy link
Contributor

arya2 commented Dec 9, 2022

@Mergifyio update

@mergify
Copy link
Contributor

mergify bot commented Dec 9, 2022

update

✅ Branch has been successfully updated

@teor2345
Copy link
Contributor Author

teor2345 commented Dec 9, 2022

There shouldn't be any Rust code in this PR, I'll fix it.

@teor2345
Copy link
Contributor Author

teor2345 commented Dec 9, 2022

It should be fixed now, I probably caused this by basing my branch off the RPC I was testing.

mergify bot added a commit that referenced this pull request Dec 9, 2022
@mergify mergify bot merged commit ac6e67d into main Dec 9, 2022
@mergify mergify bot deleted the fix-rpc-diff-order branch December 9, 2022 05:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rpc Area: Remote Procedure Call interfaces C-bug Category: This is a bug 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