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

Implement Debug trait for Scalar type #578

Merged
merged 1 commit into from
Feb 2, 2023

Conversation

arik-so
Copy link
Contributor

@arik-so arik-so commented Feb 2, 2023

Currently, Scalar types do not implement the Debug trait, whereas most other types in the library do. Besides that being an upstream requirement for us, I believe it would also be quite useful for users of that type.

Also implements the Index traits for Scalar.

@apoelstra
Copy link
Member

This looks good to me, but could you run cargo +nightly fmt on it? It won't pass CI otherwise.

@arik-so arik-so force-pushed the make-scalars-debuggable branch from 62095f4 to 8ed8cac Compare February 2, 2023 17:34
@arik-so
Copy link
Contributor Author

arik-so commented Feb 2, 2023

Sure, done!

Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK 8ed8cac

@apoelstra apoelstra merged commit 8603719 into rust-bitcoin:master Feb 2, 2023
@@ -23,6 +23,7 @@ use crate::constants;
#[allow(missing_debug_implementations)]
Copy link
Member

Choose a reason for hiding this comment

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

This line could have been removed in this patch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm happy to open a followup PR. Shall I?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I went ahead and did to save time: #579. If it's just a nuisance PR due to its size, I'm happy to close it.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks man!

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