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

feat: refactor Commitment trait to return Vec<Self> instead of mutating slice #184

Merged
merged 2 commits into from
Sep 26, 2024

Conversation

Gmin2
Copy link
Contributor

@Gmin2 Gmin2 commented Sep 25, 2024

Rationale for this change

Mentioned in #165

What changes are included in this PR?

  • Modified the Commitment trait's compute_commitments method to return Vec<Self> instead of mutating a pre-allocated slice.
  • Updated implementations for NaiveCommitment, DoryCommitment, and DynamicDoryCommitment to align with the new trait definition.

Fixes #165
/claim #165

@Dustin-Ray
Copy link
Contributor

Dustin-Ray commented Sep 26, 2024

@Gmin2 Thanks a bunch for the effort on this, can you please check our CI and ensure that clippy and cargo fmt have been run as currently these appear to be failing in the CI. Thank you! Feel free to request a review when these items are addressed.

Copy link
Contributor

@JayWhite2357 JayWhite2357 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. Just run cargo fmt and clean up the clippy lint, and it will be good to go!
We run both of the following in CI:

cargo fmt --all -- --config imports_granularity=Crate,group_imports=One
cargo clippy --all-targets --all-features -- -D warnings

@Gmin2 Gmin2 requested a review from JayWhite2357 September 26, 2024 04:18
@Gmin2
Copy link
Contributor Author

Gmin2 commented Sep 26, 2024

Just run cargo fmt and clean up the clippy lint, and it will be good to go!
We run both of the following in CI:

@JayWhite2357 , done with the changes

@JayWhite2357
Copy link
Contributor

/approve

Copy link

algora-pbc bot commented Sep 26, 2024

@JayWhite2357: The claim has been successfully added to reward-all. You can visit your dashboard to complete the payment.

Copy link
Contributor

@JayWhite2357 JayWhite2357 left a comment

Choose a reason for hiding this comment

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

LGTM
Thanks!

@JayWhite2357 JayWhite2357 merged commit e40362a into spaceandtimelabs:main Sep 26, 2024
10 checks passed
Copy link

🎉 This PR is included in version 0.22.15 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Return result from Commitment::compute_commitments rather than mutate parameter.
3 participants