-
Notifications
You must be signed in to change notification settings - Fork 14
feat: add type declarations for lua 5.0 #59
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not in favour of splitting these functions out of the core directory and into 15 different version specific files, this seems like a nightmare to maintain. Please keep the core files as they are and reconsider how to do 5.0 specific stuff.
But the functions I removed from core are simply not available in 5.0, so it doesn't make sense to keep them there. Anything in core must be available to all versions, if I've understood the setup correctly. You can categorize the new additions into just two domains: "5.0" and "5.1 and above." The only reason I created so many new files was to eliminate as much duplicate code as possible, as is done in the existing declarations. If we were to accept some amount of duplication, that should allow me to simplify things dramatically. |
I am okay with accepting some duplication, splitting into 5.0 and 5.1+ sounds like a good idea. If possible try keeping the current declarations as they are as much as possible. |
This PR should be ready for review again. How does this look? I've moved the declarations for 5.0 into entirely separate files. |
Note that Lua 5.0 breaks some of our assumptions about which features are common to all versions - see the deletions in core/. That means the tests also need to be updated.