-
Notifications
You must be signed in to change notification settings - Fork 26
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
Named args #174
Named args #174
Conversation
mbroughani81
commented
Aug 9, 2023
- rename the variables used in desugaring.
- delete unnecessary logs
- add test
Before making a formal PR(i.e., non-draft), please:
|
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.
Actually there are still a bunch of things to improve before we can merge this.
You need to understand that err
reports an error message to the programmer. Doing things like err("match error", f.toLoc)
makes zero sense.
I noticed that your latest commits removed some test cases, for example 84995d0. There are already very few tests, so please do not make it worse by reducing the coverage further! ;^) |
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.
Ok, I think we can merge now. Please address the remaining comments and I'll make the squash-merge later.