-
Notifications
You must be signed in to change notification settings - Fork 1
feat(lltz_codegen): smartpy compatible #44
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
base: main
Are you sure you want to change the base?
Conversation
da9b972 to
d5beb31
Compare
|
After briefly going through it, this PR looks good to me. However as main is currently the branch that LIGO uses as the stable one, I would suggest that we don't merge this to main but instead into some dev branch from which we less frequently merge into main(and when doing so testing LIGO tests/SmartPY tests and also LLTZ tests - ideally we should make some nice CI for this too). Better naming would be relase/dev branches, but I would suggest making this change later in order to not block Eduardo with the release of LLTZ in LIGO (only critical bugfixes should be merged to main now). |
|
Please also add manual testing section to the PR - testing on the SmartPy side and LLTZ side |
but what counts as critical - from smartpy point of view renaming these modules in LLTZ is critical as the alternative - renaming in smartpy - would involve changing most of the core compiler modules |
yes, I'm not saying that you can't use it or that it shouldn't be merged. But right now LIGO is ready to have LLTZ released in next few days via an optional --lltz-ir flag and it uses main as the stable branch of LLTZ. In the case of SmartPy, I assume that those changes aren't going to be released as soon, so I think it could use dev branch for now. Likely, you will need to make several more changes relevant for SmartPy, so I would prefer merging them to main later and updating it for next release in LIGO. by manual testing I mean running dune test --root=. in LLTZ to test that LLTZ tests pass and whatever command you use for testing in smartpy. Its fine if you don't inlcude testing from LIGO if you merge it into a dev branch, but once dev is being merged to LIGO, it would also need to be tested from LIGO side. |
brilliant, thanks. That sounds fine |
cd46737 to
2014072
Compare
ebfc5c4 to
486715b
Compare
486715b to
bed4222
Compare
a3e8d02 to
f495ffd
Compare
4bcbdf9 to
3a64ddf
Compare
3a64ddf to
0036043
Compare
0036043 to
1f4cc5e
Compare
Changes to make lltz compatible with smartpy env.
Not urgent but thought I'd open the MR anyway