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

fix: better assertions on function parameters #96

Merged
merged 6 commits into from
Mar 21, 2024

Conversation

sebffischer
Copy link
Collaborator

@sebffischer sebffischer commented Mar 18, 2024

baby steps, but solves #95

There is still some stuff open but for that I think I need to dig a little deeper into the parser (#95)

@dgkf
Copy link
Owner

dgkf commented Mar 19, 2024

This is looking great.

There is still some stuff open but for that I think I need to dig a little deeper into the parser

Personally, I think this is probably ready to be incorporated with just the bit of polish as mentioned above, but let me know when you feel like it's complete for merging.

I think we can also address #95 here in a pretty straightforward way, but I might be overoptimistic in assuming we can just check for a missing "key".

@sebffischer
Copy link
Collaborator Author

Thanks, your suggestion worked! :) From my side it is good to be merged.

@sebffischer
Copy link
Collaborator Author

Also I appreciate your quick responses and kind feedback! This makes it fun to contribute here 🥳

@dgkf
Copy link
Owner

dgkf commented Mar 21, 2024

Just did some light touch-ups to the MR:

  • Added check for when the rest-args experiment is enabled, in which case the ..args should be treated like an ellipsis parameter
  • Added a few new error variants so that we have more explicit error types. Although not implemented yet, my hope is that by having more strictly typed errors they can be more easily localized eventually.
  • Did a chore for myself by converting old localization names to ISO639 codes

Otherwise everything was in great shape already. Thanks for working on this!

@dgkf dgkf merged commit 67bbae4 into dgkf:main Mar 21, 2024
12 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.

2 participants