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 error types #89

Merged
merged 7 commits into from
Nov 11, 2024
Merged

Fix error types #89

merged 7 commits into from
Nov 11, 2024

Conversation

idky137
Copy link
Contributor

@idky137 idky137 commented Nov 2, 2024

Fixes the errors returned for get_block, get_block_range, get_nullifiers, get_nullifiers_range RPCs.

@idky137 idky137 added bug Something isn't working ZGM1 Issues that need to be resolved for the completion of the Zaino dev grant milestone 1 labels Nov 2, 2024
@idky137 idky137 marked this pull request as ready for review November 2, 2024 13:30
This was referenced Nov 2, 2024
Copy link
Contributor

@Oscar-Pepper Oscar-Pepper left a comment

Choose a reason for hiding this comment

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

im not sure if i consider the error stuff blocking but i think it could be improved. the "let _ =" i believe is a mistake here and i consider blocking, we should not have wrapped results lingering around IMO

zaino-fetch/src/jsonrpc/connector.rs Show resolved Hide resolved
{
break;
if height > chain_height {
let _ = channel_tx
Copy link
Contributor

Choose a reason for hiding this comment

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

sometimes the compliler asks to put "let _ = " but i think this is bad advice. sometimes its useful to use this to keep something in scope but i still prefer "let _unused_name_of_thing =" in stead of just underscore for that. in this case i believe you need to actually handle your result with a match and error handling or atleast an expect if it cant fail

Copy link
Contributor Author

Choose a reason for hiding this comment

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

change added in 724ef3f

{
break;
if height > chain_height {
let _ = channel_tx
Copy link
Contributor

Choose a reason for hiding this comment

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

see comment above about "let _ ="

Copy link
Contributor Author

Choose a reason for hiding this comment

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

change added in 724ef3f

This was referenced Nov 6, 2024
Copy link
Contributor

@Oscar-Pepper Oscar-Pepper left a comment

Choose a reason for hiding this comment

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

Great!

@Oscar-Pepper Oscar-Pepper merged commit 48eb1bd into zingolabs:dev Nov 11, 2024
6 of 12 checks passed
This was referenced Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ZGM1 Issues that need to be resolved for the completion of the Zaino dev grant milestone 1
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants