Skip to content

subprocess: replace __USING_WINDOWS__ with WIN32#34733

Merged
hebasto merged 1 commit intobitcoin:masterfrom
kevkevinpal:useWIN32
Mar 5, 2026
Merged

subprocess: replace __USING_WINDOWS__ with WIN32#34733
hebasto merged 1 commit intobitcoin:masterfrom
kevkevinpal:useWIN32

Conversation

@kevkevinpal
Copy link
Contributor

Summary

Motivated by #34385 (review)

In subprocess.h we are now renaming __USING_WINDOWS__ with WIN32

In the rest of the codebase, we are using WIN32, so it makes sense to update subprocess.h to match that.


Use the following grep to assert there is no __USING_WINDOWS__ in the codebase

grep -nri --exclude-dir=build "WIN32" ./ -I
rep -nri --exclude-dir=build "__USING_WINDOWS__" ./ -I

@DrahtBot
Copy link
Contributor

DrahtBot commented Mar 4, 2026

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK sedited, hebasto

If your review is incorrectly listed, please copy-paste <!--meta-tag:bot-skip--> into the comment that the bot should ignore.

Copy link
Contributor

@sedited sedited left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK bff8a7a

@sedited sedited requested a review from hebasto March 5, 2026 10:24
Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concept ACK on diverting from the upstream project for the following reasons:

  1. It targets C++11, whereas we are using C++20.
  2. We only need a subset of its functionality.

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK bff8a7a, I have reviewed the code and it looks OK.

@hebasto hebasto merged commit 7e91060 into bitcoin:master Mar 5, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants