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

Test sv2 ffi interface #30

Merged
merged 40 commits into from
Aug 26, 2021

Conversation

rrybarczyk
Copy link
Collaborator

No description provided.

Copy link
Collaborator

@Fi3 Fi3 left a comment

Choose a reason for hiding this comment

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

There are 2 issues related to the PR that should be added as they are not a priority but still very
relevant:

First issue:

The below should compile:
cargo test --features prop_test --features with_serde

Second issue

When implementing Arbitrary we should also implement shrink.

@rrybarczyk
Copy link
Collaborator Author

When I run $ cargo test --features prop_test, I get the following warning for every decoder.next_frame();:

warning: unused `Result` that must be used
   --> protocols/v2/sv2-ffi/src/lib.rs:644:9
    |
644 |         decoder.next_frame();
    |         ^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `#[warn(unused_must_use)]` on by default
    = note: this `Result` may be an `Err` variant, which should be handled

Do you have a suggestion on how I can fix these warnings?

@Fi3
Copy link
Collaborator

Fi3 commented Aug 12, 2021

When I run $ cargo test --features prop_test, I get the following warning for every decoder.next_frame();:

warning: unused `Result` that must be used
   --> protocols/v2/sv2-ffi/src/lib.rs:644:9
    |
644 |         decoder.next_frame();
    |         ^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `#[warn(unused_must_use)]` on by default
    = note: this `Result` may be an `Err` variant, which should be handled

Do you have a suggestion on how I can fix these warnings?

Did you try let _ = decoder.next_frame(); If it do not work just put #[allow(unused_must_use)] no need to handle the result as we know that it is an error and we know that is the correct behavior.

@rrybarczyk
Copy link
Collaborator Author

When I run $ cargo test --features prop_test, I get the following warning for every decoder.next_frame();:

warning: unused `Result` that must be used
   --> protocols/v2/sv2-ffi/src/lib.rs:644:9
    |
644 |         decoder.next_frame();
    |         ^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `#[warn(unused_must_use)]` on by default
    = note: this `Result` may be an `Err` variant, which should be handled

Do you have a suggestion on how I can fix these warnings?

Did you try let _ = decoder.next_frame(); If it do not work just put #[allow(unused_must_use)] no need to handle the result as we know that it is an error and we know that is the correct behavior.

Yes that fixed it! Ty.

@rrybarczyk rrybarczyk force-pushed the test-sv2-ffi-interface branch from 260a7ce to 55b5b98 Compare August 20, 2021 15:41
UnidenifiedUser and others added 25 commits August 26, 2021 18:39
@rrybarczyk rrybarczyk force-pushed the test-sv2-ffi-interface branch from 093a74b to 4c1679d Compare August 26, 2021 18:39
@rrybarczyk rrybarczyk merged commit f470392 into stratum-mining:main Aug 26, 2021
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.

3 participants