-
Notifications
You must be signed in to change notification settings - Fork 67
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
Make find_binding match annotated declarations #425
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.
Thanks @hferee for opening #424 then proposing a patch!
Could you rename your PR title so that it is more descriptive? (it's enough if the Fixes #424
string is only in the commit message or PR description; while the PR title will appear in the Merge commit and should be self-contained)
@yurug do you know whether it'd be necessary to add a small test here: https://github.com/ocaml-sf/learn-ocaml/tree/master/tests ?
At first sight, I'd say no… (as no risk of regression is expected 😅)
That would not hurt: @hferee, would you mind doing it, please? |
Here you go! Please let me know if more tests are necessary. |
Thanks! |
Thanks @hferee! LGTM as well. So let's merge your PR after setting its milestone to 0.13. |
BTW @hferee, you may want to checkout the page https://github.com/settings/emails Otherwise, GitHub is currently unable to associate your commits and your user account: But if you fix this, you'll see your name appear in https://github.com/ocaml-sf/learn-ocaml/graphs/contributors |
Well, sure. Not that I'm doing this for fame 😄 |
The honor is on our side! |
Fixes #424