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

Update mlua 10.0 and deprecate rlua #90

Merged
merged 2 commits into from
Nov 13, 2024
Merged

Conversation

jj136975
Copy link
Contributor

@jj136975 jj136975 commented Nov 1, 2024

MLua just got an update to version 10.0 that fully remove 'lua lifetime and much more.
RLua version 20.0 is now deprecated and merged into MLua (rlua re-export mlua modules).

Maybe it's a good time to bump tealr version

Update example mains
Update other dependencies
Remove rlua (deprecated in favor to mlua)
Small code cleanups
Fix tests
@lenscas
Copy link
Owner

lenscas commented Nov 1, 2024

The title says "deprecate rlua" but it seems to be entirely removed instead?

Edit: not that I disagree with deprecating rlua, as yes, the crate itself is deprecated. But I do think that releasing one version where it is properly marked as deprecated is a better warning than straight up removal right of the bat?

@jj136975
Copy link
Contributor Author

jj136975 commented Nov 1, 2024

I've tried upgrading only mlua to 10.0 and let rlua to 19.0 but the implementation is way too different now.
Maybe you can make a version with some deprecation warning and remove rlua entirely with a new major like tealr 0.10.0 (it will follow mlua major)

@lenscas
Copy link
Owner

lenscas commented Nov 1, 2024

Fair. I will look at it more during the weekend.

@lenscas
Copy link
Owner

lenscas commented Nov 3, 2024

Didn't have nearly as much time as I wanted. So, didn't really get to it.

One question though: Can it be made such that the old path is still valid? I have a private version of tealr where I play with piccolo support. So, eventually (hopefully at least) the tealr::mlu::* path will come back anyway and having it also reduces the amount of churn that needs to happen now.

I am fine with the movement of all the code itself though. It will probably be quite a while before the piccolo backend can be merged proper and until then there is no reason to pretend there is another backend.

@jj136975
Copy link
Contributor Author

jj136975 commented Nov 3, 2024

I don't realy understand your question. Maybe change the destination branch of this PR to a new one, then apply any changes you want and submite a new PR.
Alternatively you can write a PR on my fork.

@lenscas
Copy link
Owner

lenscas commented Nov 13, 2024

Disregard my last comment. No idea why I thought you got rid of the mlu module.

Anyway, managed to look better at the code. Fixed the problems caught in CI and ported tealsql over to it. Still need to make a couple of changes and then I am going to see if tealr_doc_gen works with the new version.

Assuming everything works then I can do a new release tomorrow.

@lenscas lenscas mentioned this pull request Nov 13, 2024
@lenscas lenscas merged commit 2a3dd23 into lenscas:master Nov 13, 2024
8 of 12 checks passed
@lenscas
Copy link
Owner

lenscas commented Nov 13, 2024

Thank you for your help in getting this done! 🎉

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