-
Notifications
You must be signed in to change notification settings - Fork 121
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
Update bindings #1011
Update bindings #1011
Conversation
looks like you have to merge |
Notably includes o1-labs/o1js-bindings#63
Do I need to declare the function in some TS file? |
No that's not it, what CI shows is a runtime error |
You need to push updated artifacts created by |
Do you still want to land this @L-as? The tests don't work, I'm pretty sure that's just because you didn't check our the bindings submodule at the latest commit of the bindings PR |
@mitschabaude Sorry for the very late response, but I don't understand why it fails honestly, neither do I understand your hypothesis, because I merely advanced the commit once to include the new functions. I looked at the error when I first pushed the commit and mentally made a note to look at it again, and at that time I assumed it was because I was missing something somewhere to properly export the function, though it works locally (under different circumstances). I wouldn't be so sure it's not buggy but if you believe it isn't then 🤷 . |
@L-as on this PR, if you click on "Files changed", then you see that in the bindings submodule, nothing in the So, to land this PR, do the following: # you are in /snarkyjs
# build updated bindings
npm run bindings
# push those to the bindings PR
cd src/bindings
git add compiled
git add MINA_COMMIT
git commit -m "update bindings"
git push
# update snarkyjs PR
cd ../..
git add src/bindings
git commit -m "bindings"
git push |
I will get back to this |
stale |
Notably includes o1-labs/o1js-bindings#63