search

Azure / azure-sdk-for-cpp

This repository is for active development of the Azure SDK for C++. For consumers of the SDK we recommend visiting our versioned developer docs at https://azure.github.io/azure-sdk-for-cpp.
MIT License
170 stars 118 forks source link

Remove 'include windows.h' from inc/* #5691

Closed antkmsft closed 3 weeks ago

antkmsft commented 3 weeks ago

In the light of #5660 (closes #5660 ?).

WinHttpTransport is the only header in sdk/**/inc/* that has #include <Windows.h>.

If we remove this, further discussion on whether to have NOMINMAX defined in public header or not becomes hypothetical.

(We still should write min and max as (min) and (max) in the sdk/**/inc/* files, and I recommend we do keep defining NOMINMAX and WIN32_LEAN_AND_MEAN in sdk/**/src/* and sdk/**/test/* files).

The PR #5682, which is currently a Draft and has #include <windows.h> in a new sdk/**/inc/* file in its latest iteration as of now, the author agreed to move #include <windows.h> into a .cpp file in the next iteration.

LarryOsterman commented 3 weeks ago

I was planning on making this change on my own, cleaning up a bunch of stuff that goes against the new guidelines (declaring types in _detail in public headers).

We can keep this PR with my feedback, or remove it.