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: workaround for LSP dependency resolution #1865

Merged
merged 10 commits into from
Jul 13, 2023
Merged

fix: workaround for LSP dependency resolution #1865

merged 10 commits into from
Jul 13, 2023

Conversation

ludamad
Copy link
Collaborator

@ludamad ludamad commented Jul 5, 2023 •
edited
Loading

Description

This is currently not ideal in form but should inform a larger fix. There was a desire to get it into master soon.

Problem*

#1838 will take some time for a larger fix, meanwhile there was a desire for a solution for the current Noir dogfooding.

Summary*

Works around issues by:

  1. Loading from main.nr or lib.nr as appropriate instead of just any entry point
  2. Filtering errors based on the the file pased
  3. Loading Nargo.toml dependencies which are used in aztec contracts

PR Checklist*

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

@ludamad
fix: workaround for #1838
0bb7b53 
This is currently not ideal in form but should inform a larger fix
@ludamad ludamad changed the title fix: workaround for #1838 fix: workaround for LSP dependency resolution Jul 5, 2023
@ludamad ludamad requested review from phated and kevaundray July 5, 2023 16:17
phated
phated previously requested changes Jul 5, 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.

I'm currently making movement on #1838 so I don't think we should merge this

@charlielye
Copy link
Contributor

@phated any timeline on your branch? any reason we can't merge this now so we can all get the benefit, and then revert/replace once your stuff is ready? would speed us up.

@ludamad ludamad closed this Jul 6, 2023
sirasistant and others added 2 commits July 6, 2023 18:31
@sirasistant @jfecher @ludamad
feat: make array indexes polymophic integers (#1877)
87baebf 
* feat: make array indexes poly int

* Update crates/noirc_frontend/src/hir/type_check/expr.rs

* Update crates/noirc_frontend/src/hir/type_check/stmt.rs

---------

Co-authored-by: jfecher <[email protected]>
@kevaundray @ludamad
Make proving and verification keys optional
f1bbdc2
@kobyhallx kobyhallx reopened this Jul 13, 2023
@kobyhallx kobyhallx self-requested a review July 13, 2023 15:16
dbanks12
dbanks12 reviewed Jul 13, 2023
View reviewed changes
Copy link

@dbanks12 dbanks12 left a comment

Choose a reason for hiding this comment

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

Tested and all appears to be working nicely in VSCode on this branch!

kobyhallx
kobyhallx approved these changes Jul 13, 2023
View reviewed changes
Copy link
Contributor

@kobyhallx kobyhallx left a comment

Choose a reason for hiding this comment

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

Approving ad temp solution, thanks Adam!

@ludamad ludamad requested a review from phated July 13, 2023 15:31
@ludamad ludamad enabled auto-merge July 13, 2023 15:33
@kobyhallx kobyhallx dismissed phated’s stale review July 13, 2023 15:56

temporarily to allow workaround

@ludamad ludamad added this pull request to the merge queue Jul 13, 2023
Merged via the queue into master with commit a8ac338 Jul 13, 2023
@ludamad ludamad deleted the ad/workaround-1838-lsp-deps branch July 13, 2023 16:10
@kevaundray kevaundray mentioned this pull request Jul 13, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dbanks12 dbanks12 dbanks12 left review comments

@kobyhallx kobyhallx kobyhallx approved these changes

@kevaundray kevaundray Awaiting requested review from kevaundray

@phated phated Awaiting requested review from phated

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

Successfully merging this pull request may close these issues.
7 participants
@ludamad @charlielye @phated @dbanks12 @kobyhallx @sirasistant @kevaundray