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: Fix nargo not showing compiler errors or warnings #1694

Merged
merged 5 commits into from
Jun 15, 2023
Merged

fix: Fix nargo not showing compiler errors or warnings #1694

merged 5 commits into from
Jun 15, 2023

Conversation

jfecher
Copy link
Contributor

@jfecher jfecher commented Jun 14, 2023

Description

Problem*

Fixes an issue where if there are any compiler errors or warnings Nargo would map them into a CompilationError without reporting them first, leading to no errors being reported.

Resolves #1645

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.

phated
phated suggested changes Jun 14, 2023
View reviewed changes
Copy link
Contributor

@phated phated left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Question about the finish_report and checking error_count

crates/nargo_cli/src/cli/compile_cmd.rs Outdated Show resolved Hide resolved
@jfecher jfecher requested a review from phated June 14, 2023 21:00
phated
phated approved these changes Jun 14, 2023
View reviewed changes
Copy link
Contributor

@phated phated left a comment

Choose a reason for hiding this comment

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

This looks great 😄

joss-aztec
joss-aztec approved these changes Jun 15, 2023
View reviewed changes
Copy link
Contributor

@joss-aztec joss-aztec left a comment

Choose a reason for hiding this comment

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

LGTM

@joss-aztec joss-aztec added this pull request to the merge queue Jun 15, 2023
Merged via the queue into master with commit 4233068 Jun 15, 2023
@joss-aztec joss-aztec deleted the jf/fix-1645 branch June 15, 2023 10:44
This was referenced Jun 15, 2023
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joss-aztec joss-aztec joss-aztec approved these changes

@phated phated phated approved these changes

@TomAFrench TomAFrench Awaiting requested review from TomAFrench

@kevaundray kevaundray Awaiting requested review from kevaundray

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

Successfully merging this pull request may close these issues.

"Failed to compile circuit" and no errors shown when compiling code with any errors or warnings
3 participants
@jfecher @phated @joss-aztec