-
Notifications
You must be signed in to change notification settings - Fork 11
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 Issue #513 #514
Fix Issue #513 #514
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 a lot for the bug fix!
I've made a suggestion to avoid deleting the second dbg_generate
.
For the CHANGELOG can you write an entry for the header "### Fixed"?
I'm not quite sure how to add tests for this fix. Maybe we shall just test all the function calling methods documented works, or just leave them, since it is really simple. |
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.
The new version looks great thanks a lot.
No need to add tests to these functions, as they are just convenience functions used internally for debugging and might change at any time.
This kind of error could have been caught in a different way, though, with something like SnoopCompile, JET, or Aqua (#216, #64) used as linters, but I haven't looked into that yet.
The CHANGELOG entry is also great, thanks
@all-contributors please add @hz-xiaxz for code |
I've put up a pull request to add @hz-xiaxz! 🎉 |
Might be good to tag a release with this PR, because precompilation of the package is broken on the current release:
|
Will do one now |
Done |
Related issues
fix #513
Closes #
Checklist