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

📝 Adding Steps for 09-recursion tutorial #796

Merged

Conversation

LuffySama-Dev
Copy link
Contributor

Hi @barriebyron ,

Good Day!!
I am creating this new PR instead of this old PR #763 as many new changes have been committed in this repo.
I have made all the recommended changes in this PR.

I am still working on the Vote and Rollup so we can keep this PR in draft for couple of days.
I will try to commit all the changes and close it by tomorrow EOD.

You can track changes here:

Changes made:

  • Tutorial made easy to follow
  • Using latest o1js version
  • Match the file structure and code base to tutorial
  • Changes made in example are tested locally

Upcoming changes:

  • Updating tutorial for rollup.ts example
  • Updating tutorial for vote.ts example
  • Updating the file structure and code according to upcoming changes in tutorial

Issues being resolve here are :

Thank you 😃

@LuffySama-Dev LuffySama-Dev requested a review from a team as a code owner January 16, 2024 12:37
Copy link

vercel bot commented Jan 16, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs2 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 13, 2024 9:27pm

Copy link

vercel bot commented Jan 16, 2024

@LuffySama-Dev is attempting to deploy a commit to the o1labs Team on Vercel.

A member of the Team first needs to authorize it.

@LuffySama-Dev
Copy link
Contributor Author

Hi @barriebyron ,

As requested in this comment #763 (comment), I have made the changes.

Can you please help to review ?

Thank You 😃

@barriebyron
Copy link
Contributor

@iregina can you do the first round of review and testing? thank you!

@iregina
Copy link
Contributor

iregina commented Feb 8, 2024

I ran your code, but came across an issue in the src/zkProgram_wrapper.ts:

src/zkProgram_wrapper.ts:9:3 - error TS2440: Import declaration conflicts with local declaration of 'ZkProgram'.

Question - @LuffySama-Dev Did you also get this issue when you ran this with o1js version 15.1?

@barriebyron barriebyron changed the title 📝 Adding Steps for 09-recurssion tutorial 📝 Adding Steps for 09-recursion tutorial Feb 8, 2024
Copy link
Contributor

@barriebyron barriebyron left a comment

Choose a reason for hiding this comment

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

The doc updates look good @LuffySama-Dev
Our DevRel @iregina is going to run through the steps and we'll get these improvements merged, thank you so much!!

@LuffySama-Dev
Copy link
Contributor Author

I ran your code, but came across an issue in the src/zkProgram_wrapper.ts:

src/zkProgram_wrapper.ts:9:3 - error TS2440: Import declaration conflicts with local declaration of 'ZkProgram'.

Question - @LuffySama-Dev Did you also get this issue when you ran this with o1js version 15.1?

Hi @iregina ,
Oh Yes, I forgot to mention this. Thanks for reminding. The error is because the function name is same as what we are importing. (Added Screenshot for reference).

Also, I think this file is not actually require, I think it is implementation for ZkProgram in proof_system.d.ts.
The error can be removed by removing the unnecessary imports, but to be honest this program seems to be incomplete, maybe we can remove it.

Screenshot 2024-02-08 at 10 23 38 PM

Screenshot 2024-02-08 at 10 21 08 PM

Thank You 😃

Copy link
Contributor

@barriebyron barriebyron left a comment

Choose a reason for hiding this comment

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

thanks @LuffySama-Dev you are very on top of things! I learned to run the code in your PR

(see me dancing?)

Copy link
Contributor

@iregina iregina left a comment

Choose a reason for hiding this comment

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

Looks good - went through this in detail with Barrie and Jack!

@barriebyron barriebyron merged commit f57a5bd into MinaProtocol:main Feb 13, 2024
2 of 3 checks 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.

4 participants