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

feat: goodbye noir-starter, hello noirenberg #89

Merged
merged 3 commits into from
Oct 1, 2024
Merged

Conversation

signorecello
Copy link
Collaborator

Problem

Noir and BB versions were hard to sync. After some discussion, we agreed that there could be a tool to check the correct bb version for the wanted noir version.

As it wouldn't live inside the noir repository or even inside the noir-lang org (check #88), we thought it could have a creative name.

Description

Starts a refactor by extending the existent npx script to do this installation and check. It also moves it to ts using the excellent bun and tsx tooling.

I reckon I went a bit overboard with the dependencies on ora but I also wanted it to look good!

Next steps

  • This repo needs to be moved as of Migrate repository to AztecProtocol #88
  • It should also be renamed
  • It could help ease some of the annoyances between nargo and bb that we have already identified and agreed. But I'll spike those and discuss internally first

@signorecello signorecello requested review from TomAFrench, Savio-Sou and a team September 19, 2024 16:17
Copy link

netlify bot commented Sep 19, 2024

Deploy Preview for noir-vite-hardhat canceled.

Name Link
🔨 Latest commit b4ca59b
🔍 Latest deploy log https://app.netlify.com/sites/noir-vite-hardhat/deploys/66fbd75fae606c0008e33cfe

@signorecello
Copy link
Collaborator Author

btw since this repo doesn't use release-please yet, you can actually already try the script with npx noirenberg as I have already published it

Copy link

socket-security bot commented Sep 19, 2024

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher

🚮 Removed packages: npm/@inquirer/[email protected], npm/@inquirer/[email protected], npm/@inquirer/[email protected], npm/@types/[email protected], npm/@types/[email protected], npm/@types/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected]

View full report↗︎

import axios from 'axios';

export async function getBbVersion(noirVersion: string) {
const url = `https://raw.githubusercontent.com/noir-lang/noir/v${noirVersion}/scripts/install_bb.sh`;
Copy link
Member

Choose a reason for hiding this comment

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

Flagging that I'm not sure if / when @TomAFrench might plan to deprecate this script from Noir, which could break this noirenberg script here.

Copy link
Member

Choose a reason for hiding this comment

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

Given how there is no good way to source Noir compatibility from within Barretenberg yet, don't think this should be blocking.

Copy link
Member

Choose a reason for hiding this comment

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

Oh yeah, this script is purely an implementation detail of the noir repo CI and we don't provide any guarantees about its stability.

@Savio-Sou
Copy link
Member

Note that hosting the noirenberg installation script in a standalone AztecProtocol repo (instead of as a part of the starter / boilerplate repo) might make more sense.

Copy link
Collaborator

@critesjosh critesjosh left a comment

Choose a reason for hiding this comment

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

Cool tool! I think you need to update the README as well.

Do you publish updates to noirenberg directly from this repo? Is this repo the best home for this tool? It feels unrelated to a Noir stater repo, and not something that I'd necessarily want to clone when starting a noir project.

@signorecello signorecello merged commit 1273f3f into main Oct 1, 2024
6 checks passed
@signorecello signorecello deleted the zpedro/0.34.0 branch October 1, 2024 15:17
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.

4 participants