-
Notifications
You must be signed in to change notification settings - Fork 35
Add tests for xmagics, os, xutils, xinterpreter #116
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
clang-tidy review says "All clean, LGTM! 👍" |
1 similar comment
clang-tidy review says "All clean, LGTM! 👍" |
sys/wait.h is for unix systems only. You will need a different header for Windows. |
clang-tidy review says "All clean, LGTM! 👍" |
unistd.h is also unix only. |
@mcbarton the handler function which has |
@tharun571 Maybe you could try one of the suggestions from this stackoverflow page where they talk about Windows equivalents https://stackoverflow.com/questions/341817/is-there-a-replacement-for-unistd-h-for-windows-visual-c . |
Also is there a way to check windows build in local? or should I push the code everytime to check? |
If you have access to a Windows machine you could run the commands in the ci. There are currently no Windows instructions in the documentation. I will add at some point. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #116 +/- ##
==========================================
+ Coverage 70.09% 75.08% +4.98%
==========================================
Files 17 17
Lines 602 602
Branches 59 59
==========================================
+ Hits 422 452 +30
+ Misses 180 150 -30 |
clang-tidy review says "All clean, LGTM! 👍" |
Converting to draft, let us know when the PR is ready for review (after maybe squashing the commits too) |
@anutosh491 this covers most of #114. Having some trouble with complete_request. Will push that later. |
clang-tidy review says "All clean, LGTM! 👍" |
clang-tidy review says "All clean, LGTM! 👍" |
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.
Looks good. Let's wait for maybe one more review before we merge this.
cc @mcbarton
No description provided.