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

Add public getter / setter func to Zebra-RPC::methods.rs #9113

Merged
merged 12 commits into from
Jan 15, 2025

Conversation

idky137
Copy link
Contributor

@idky137 idky137 commented Jan 13, 2025

Motivation

Adds public getter and setter functionality to RPC method responses in Zebra-RPC to enable Zaino to construct and deconstruct these types.

Specifications & References

Solution

Add public getter and setter methods to structs in Zebra-RPC::methods.rs

Tests

Follow-up Work

PR Author's Checklist

  • The PR name will make sense to users.
  • The PR provides a CHANGELOG summary.
  • The solution is tested.
  • The documentation is up to date.
  • The PR has a priority label.

PR Reviewer's Checklist

  • The PR Author's checklist is complete.
  • The PR resolves the issue.

@idky137 idky137 requested a review from a team as a code owner January 13, 2025 16:32
@idky137 idky137 requested review from arya2 and removed request for a team January 13, 2025 16:32
arya2
arya2 previously approved these changes Jan 14, 2025
Copy link
Contributor

@arya2 arya2 left a comment

Choose a reason for hiding this comment

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

Looks good to me, thank you for this change.

I left a few optional suggestions for cleanup.

zebra-rpc/src/methods.rs Outdated Show resolved Hide resolved
zebra-rpc/src/methods.rs Outdated Show resolved Hide resolved
zebra-rpc/src/methods.rs Outdated Show resolved Hide resolved
@arya2
Copy link
Contributor

arya2 commented Jan 14, 2025

@Mergifyio update

Copy link
Contributor

mergify bot commented Jan 14, 2025

update

❌ Mergify doesn't have permission to update

For security reasons, Mergify can't update this pull request. Try updating locally.
GitHub response: refusing to allow a GitHub App to create or update workflow .github/workflows/cd-deploy-nodes-gcp.patch-external.yml without workflows permission

@arya2
Copy link
Contributor

arya2 commented Jan 14, 2025

@idky137 By the way, it would also be fine to make RPC request/response type fields public in the future if that's easier.

This PR will need the latest changes on main to pass CI.

@idky137
Copy link
Contributor Author

idky137 commented Jan 15, 2025

Oh cool I wasn't sure if you wanted to keep fields private. I think this will be everything we need publicised but I will keep that in mind i the future!

Copy link
Contributor

@arya2 arya2 left a comment

Choose a reason for hiding this comment

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

Thank you!

mergify bot added a commit that referenced this pull request Jan 15, 2025
@mergify mergify bot merged commit 43869bf into ZcashFoundation:main Jan 15, 2025
86 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants