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

fix: regression end to end test #1965

Merged
merged 21 commits into from
Jul 21, 2023
Merged

fix: regression end to end test #1965

merged 21 commits into from
Jul 21, 2023

Conversation

kevaundray
Copy link
Contributor

Description

Problem*

Resolves #1800

Summary*

Documentation

  • This PR requires documentation updates when merged.

    • I will submit a noir-lang/docs PR.
    • I will request for and support Dev Rel's help in documenting this PR.

Additional Context

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@kevaundray kevaundray changed the base branch from master to kw/fix-9-conditional July 18, 2023 23:20
@kevaundray
Copy link
Contributor Author

This is based off of #1951

Base automatically changed from kw/fix-9-conditional to master July 19, 2023 20:55
kevaundray and others added 3 commits July 19, 2023 22:06
@kevaundray @TomAFrench
Update crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/gener…
28786e9 
…ated_acir.rs

Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
@kevaundray
change unwrap to expect
b6a0bc0
@kevaundray
make mul_with_witness crate visible
5a8fe0f 
remove bad merge
TomAFrench
TomAFrench reviewed Jul 20, 2023
View reviewed changes
Copy link
Member

@TomAFrench TomAFrench left a comment

Choose a reason for hiding this comment

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

We can be more liberal with mul_with_witness here to abstract away the witness creation (and potentially avoid it).

crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs Outdated Show resolved Hide resolved
crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs Outdated Show resolved Hide resolved
crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs Outdated Show resolved Hide resolved
@TomAFrench
Copy link
Member

Addressed my review comments in #1989 as the changes have already hit master.

@kevaundray kevaundray requested review from jfecher and TomAFrench July 20, 2023 22:45
@kevaundray kevaundray enabled auto-merge July 21, 2023 00:17
@kevaundray
Copy link
Contributor Author

@guipublic can you also give an explicit approval for this PR!

@kevaundray kevaundray added this pull request to the merge queue Jul 21, 2023
TomAFrench
TomAFrench approved these changes Jul 21, 2023
View reviewed changes
Copy link
Member

@TomAFrench TomAFrench left a comment

Choose a reason for hiding this comment

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

Lgtm

@TomAFrench TomAFrench disabled auto-merge July 21, 2023 03:16
@TomAFrench TomAFrench removed this pull request from the merge queue due to a manual request Jul 21, 2023
@TomAFrench
Copy link
Member

Removing from queue as kev asked for guillaume to take a look. Feel free to kick off merge again.

@guipublic guipublic added this pull request to the merge queue Jul 21, 2023
Merged via the queue into master with commit 59f92e3 Jul 21, 2023
@guipublic guipublic deleted the kw/fix-regression branch July 21, 2023 08:54
@kevaundray kevaundray mentioned this pull request Jul 21, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@guipublic guipublic guipublic approved these changes

@TomAFrench TomAFrench TomAFrench approved these changes

@jfecher jfecher Awaiting requested review from jfecher

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

Successfully merging this pull request may close these issues.

Could not satisfy all constraints for regression test
4 participants
@kevaundray @TomAFrench @jfecher @guipublic