-
Notifications
You must be signed in to change notification settings - Fork 13
Feature #845 x asl system #846
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
MichaelStritt
left a comment
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.
@HenkMutsaerts: code looks good to me and I really don't like to be the guy who says this, but we agreed during the v1.8.0 release to either show examples of tests you did for the changes or to add a code example on how to test it. This will help us to have a less stressful release process next time.
Besides that: great header, looks awesome and makes the docu process a lot easier.
|
Thanks! Good idea, do you something like : To test this PR:
|
Exactly, something like this. I tested these cases: That stuff seems to work fine 👍 |
MichaelStritt
left a comment
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.
Had to do some minor changes to make it work on windows as well 👍
Linked issue
Closes #845 (partly)
How to test
See header of
xASL_system. We should be able to merge this now, and improve this function later (according to #845), so then the other Wishlist contents from #845 can then be moved to a new issue :)Comments
Optional: add helpful comments for the reviewers here