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.

Sign up for GitHub

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

Jump to bottom

47 Code Quality PiFac #417

Merged
merged 15 commits into from
Jul 6, 2023
Merged

47 Code Quality PiFac #417

merged 15 commits into from
Jul 6, 2023

Conversation

hridambasu
Copy link

Closes #47

@hridambasu hridambasu requested a review from marsella June 14, 2023 06:38
marsella
marsella suggested changes Jun 15, 2023
View reviewed changes
Copy link

@marsella marsella left a comment

Choose a reason for hiding this comment

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

Looking pretty good so far. I think it would be helpful to have a working session to discuss potential names for things together.

src/zkp/pifac.rs Outdated Show resolved Hide resolved
src/zkp/pifac.rs Outdated Show resolved Hide resolved
src/zkp/pifac.rs Outdated Show resolved Hide resolved
src/zkp/pifac.rs Show resolved Hide resolved
src/zkp/pifac.rs Outdated Show resolved Hide resolved
src/zkp/pifac.rs Outdated Show resolved Hide resolved
marsella
marsella suggested changes Jun 20, 2023
View reviewed changes
Copy link

@marsella marsella left a comment

Choose a reason for hiding this comment

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

I think all of these comments are about documentation :)

src/zkp/pifac.rs Outdated Show resolved Hide resolved
src/zkp/pifac.rs Outdated Show resolved Hide resolved
src/zkp/pifac.rs Outdated Show resolved Hide resolved
src/zkp/pifac.rs Outdated Show resolved Hide resolved
src/zkp/pifac.rs Outdated Show resolved Hide resolved
src/zkp/pifac.rs Outdated Show resolved Hide resolved
src/zkp/pifac.rs Outdated Show resolved Hide resolved
src/zkp/pifac.rs Outdated Show resolved Hide resolved
src/zkp/pifac.rs Outdated Show resolved Hide resolved
src/zkp/pifac.rs Outdated Show resolved Hide resolved
marsella
marsella reviewed Jun 22, 2023
View reviewed changes
src/protocol.rs Outdated Show resolved Hide resolved
src/zkp/pifac.rs Outdated Show resolved Hide resolved
src/zkp/pifac.rs Outdated Show resolved Hide resolved
src/zkp/pifac.rs Outdated Show resolved Hide resolved
src/zkp/pifac.rs Show resolved Hide resolved
@hridambasu hridambasu force-pushed the 47-code-qual-pifac branch from 058d4f2 to 2cd5c82 Compare June 28, 2023 13:40
marsella
marsella reviewed Jun 28, 2023
View reviewed changes
src/zkp/pifac.rs Outdated Show resolved Hide resolved
src/zkp/pifac.rs Show resolved Hide resolved
src/zkp/pifac.rs Outdated Show resolved Hide resolved
src/zkp/pifac.rs Outdated Show resolved Hide resolved
marsella
marsella reviewed Jun 28, 2023
View reviewed changes
Copy link

@marsella marsella 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 still some unresolved comments from previous reviews; they need to be addressed before this can be approved.

marsella
marsella approved these changes Jul 6, 2023
View reviewed changes
Copy link

@marsella marsella 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, thank you!

src/zkp/pifac.rs Show resolved Hide resolved
@marsella marsella mentioned this pull request Jul 6, 2023
Merged
@hridambasu hridambasu merged commit 3349626 into main Jul 6, 2023
@hridambasu hridambasu deleted the 47-code-qual-pifac branch July 6, 2023 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marsella marsella marsella approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Improve code quality for PiFacProof
2 participants
@hridambasu @marsella