-
Notifications
You must be signed in to change notification settings - Fork 205
fix: define S_ISREG on Windows #1061
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: define S_ISREG on Windows #1061
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1061 +/- ##
==========================================
- Coverage 25.14% 25.14% -0.01%
==========================================
Files 570 570
Lines 234225 234225
Branches 41283 41285 +2
==========================================
- Hits 58905 58903 -2
- Misses 175320 175322 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
perazz
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.
LGTM @LaplaceSoda, thank you. Can I ask you to remove the style/format changes, so this PR better highlights the new preprocessor definitions?
- detect whether POSIX-style macros exist before using them - fall back to the MSVC _S_IF* constants when needed
9827616 to
bb86830
Compare
Done! I've removed the style/format changes and kept only the essential preprocessor definitions. Should be much clearer now. |
perazz
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.
Will wait a bit longer and then merge, if there are no further comments. Thank you @LaplaceSoda.
jvdp1
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.
thank you @LaplaceSoda . LGTM.
jalvesz
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.
LTGM @LaplaceSoda, thanks for this contribution!
Uh oh!
There was an error while loading. Please reload this page.