Skip to content

Conversation

pmikolajczyk41
Copy link
Contributor

@pmikolajczyk41 pmikolajczyk41 commented Sep 1, 2022

Description

When working on the exercises I encountered two annoying things:

  • cargo was continually complaining about some small problems (even after completing exercise)
  • 2nd exercise ('Verifying a single transaction') couldn't have been tested separately from the 3rd one

This PR:

  • calms down cargo clippy as much as possible (i.e. after completing the exercises, there should be no warnings)
  • introduces UnaryRollup which is a simplified version of Rollup - this one serves for testing 2nd exercise
  • adds a unit test for the 2nd exercise using UnaryRollup
  • fixes rendering of some READMEs

Checklist:

  • Targeted PR against correct branch (main)
  • Linked to Github issue with discussion and accepted design OR have an explanation in the PR that describes this work.
  • Wrote unit tests (n/a - typos and unit test itself)
  • Updated relevant documentation in the code
  • Added a relevant changelog entry to the Pending section in CHANGELOG.md (n/a - seems to be unmaintained)
  • Re-reviewed Files changed in the Github PR explorer

@weikengchen
Copy link
Member

Let me clean it up a bit, apparently the remaining tests have been failing for more than one year.

@pmikolajczyk41
Copy link
Contributor Author

@weikengchen I'm afraid that unused variable is the thing that we should ignore in CI (or prepend corresponding object names with _), at least for such (tutorial) repository

I'm not so sure about no-std checks though

@weikengchen
Copy link
Member

Let me merge it first. The no-std check is pretty weird, and I believe a larger refactoring is likely needed for this to be compatible with 0.4.0.

@weikengchen weikengchen merged commit 5d3a902 into arkworks-rs:main Sep 12, 2022
@pmikolajczyk41 pmikolajczyk41 deleted the piomiko/minor-improvements branch September 12, 2022 05:39
@pmikolajczyk41
Copy link
Contributor Author

@weikengchen thank you!

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