-
Notifications
You must be signed in to change notification settings - Fork 38.7k
compat: move (win) S_* defines into bdb #26832
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
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsNo conflicts as of last run. |
|
Concept ACK. |
77d6f3c to
cc16ab1
Compare
|
Rebased on #27098. |
cc16ab1 to
cf0d86e
Compare
|
I feel like I'm missing some context here. Why move these out of compat? There are also other symbols like |
My plan was to split compat up further, so we didn't have the singular compat.h, with everything thrown in. |
|
Light code review ACK cf0d86ed15041822e2a0a55ddf231af7809a9e66 |
We've already used it unguarded in `httpserver.cpp` for years, with no build issues.
cf0d86e to
54e4061
Compare
|
Rebased for #27254. |
hebasto
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.
ACK 54e4061, I have reviewed the code and it looks OK.
nit: Two commits "refactor: don't avoid sys/types.h on when building for Windows" and "compat: move (win) S_* defines into bdb" were squashed but the commit message of the latter was lost.
|
re-ACK 54e4061 |
54e4061 refactor: don't avoid sys/types.h on when building for Windows (fanquake) Pull request description: This is the only place these defines are used. They may also be available when building for Windows. `sys/stat.h` is available, and we already use it unguarded in other code. So move the defines into bdb, after the stat.h include, and remove compat from bdb.cpp. ACKs for top commit: TheCharlatan: re-ACK 54e4061 hebasto: ACK 54e4061, I have reviewed the code and it looks OK. Tree-SHA512: b75bb120654b4dec9ccc83aa407ee1a50969fec92f196a3722ec51282b91ac50e455af04f07211f3e93270ab83660f1efdeef43928b44b1e4296f6b06ea807c8
This is the only place these defines are used. They may also be available when building for Windows.
sys/stat.his available, and we already use it unguarded in other code. So move the defines into bdb, after the stat.h include, and remove compat from bdb.cpp.