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.

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

Bring this package up to date to use with the new noir/nargo #1

Merged
merged 6 commits into from
Feb 12, 2024

Conversation

LogvinovLeon
Copy link
Contributor

@LogvinovLeon LogvinovLeon commented Feb 2, 2024

Currently - it's impossible to use this package as a dependency because of a couple reasons:

[tooling/nargo_toml/src/lib.rs:130] name.parse() = Err(
    "Package names must be non-empty and cannot contain hyphens",
)
Invalid package name `noir-u2b` found in /Users/leonidlogvinov/nargo/github.com/colinnielsen/noir-u2bv0.3.0/Nargo.toml
  • There is no package type specified
  • All functions are private
  • Library crates should have lib.nr instead of main.nr
  • Compiler version is fixed to 0.8.0

This PR fixes that and makes it possible to import this lib into modern projects

@LogvinovLeon LogvinovLeon changed the title Remove a hyphen from the package name Bring this package up to date to use with the new noir/nargo Feb 2, 2024
@LogvinovLeon
Copy link
Contributor Author

@colinnielsen Please let me know if there is anything blocking this PR from merging

@colinnielsen
Copy link
Owner

@colinnielsen Please let me know if there is anything blocking this PR from merging

looks great - thanks!

@colinnielsen
Copy link
Owner

@colinnielsen Please let me know if there is anything blocking this PR from merging

actually - looks like CI is failing, do you mind bumping the version in CI?

@LogvinovLeon
Copy link
Contributor Author

I can't run CI on this PR, but it passes now in my fork

@colinnielsen
Copy link
Owner

I can't run CI on this PR, but it passes now in my fork

passes! merging now, thanks

@colinnielsen colinnielsen merged commit a1398b7 into colinnielsen:main Feb 12, 2024
1 check passed
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