[WIN] Use NOMINMAX consistently
Created attachment 187034 [details] Patch
Comment on attachment 187034 [details] Patch Attachment 187034 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/16431080
Created attachment 187039 [details] Patch
Since it's a duplicate of https://bugs.webkit.org/show_bug.cgi?id=73563, please look the the commtents there. I'd define it via buildsystem and remove it from all config.h files
(In reply to comment #4) > Since it's a duplicate of https://bugs.webkit.org/show_bug.cgi?id=73563, please look the the commtents there. I'd define it via buildsystem and remove it from all config.h files I'm sure Adam had a good reason behind his comment, but since we would have at least 3 build systems to adjust for this I would prefer to keep it in config.h. I believe that config.h should define what the code expects/needs from a platform, while the build system should take care of smaller details of the configuration and system the binary will run on.
Though there is already a NOMINMAX definition in: Source/JavaScriptCore/JavaScriptCore.vcproj/testapi/testapiCommon.vsprops Source/JavaScriptCore/JavaScriptCore.vcxproj/testapi/testapiCommon.props Source/cmake/OptionsWindows.cmake Tools/DumpRenderTree/win/ImageDiffCommon.vsprops Tools/DumpRenderTree/win/ImageDiffWinCairoCommon.vsprops Simon, what do you think?
(In reply to comment #5) > I believe that config.h should define what the code expects/needs from a platform, while the build system should take care of smaller details of the configuration and system the binary will run on if we have _ONE_ config.h i agree, but since that's not the case i'd prefere the build system option. Otherwise you should move the other defines into the config.h files too.
Can this code be moved to Platform.h ? If it can that I find Platform.h a good place for it.
I don't mind trying the build system option then. That would mean removing the #define min from config.h altogether.
(In reply to comment #9) > I don't mind trying the build system option then. That would mean removing the #define min from config.h altogether. Unless this is a technical necessity for the Window platform/build system, moving this definition to the build system is not the general direction that the project seems to prefer. I similar (bot not exactly the same) matter - see bug 108191 .
(In reply to comment #10) > (In reply to comment #9) > > I don't mind trying the build system option then. That would mean removing the #define min from config.h altogether. > > Unless this is a technical necessity for the Window platform/build system, moving this definition to the build system is not the general direction that the project seems to prefer. I similar (bot not exactly the same) matter - see bug 108191 . The difference is, that in that case you define a variable, which isn't used outside of WebKit, but NOMINMAX is required for a system header, which can be included at any time, where Platform.h can be too late. I completely agree that the build system isn't the best direction, but unless we have a "commonconfig.h", which gets includes by all other config.h, I still prefere the build system option (for now).
Comment on attachment 187039 [details] Patch I don't think this patch is needed anymore, now that NOMINMAX has been added to the build property sheets.
*** This bug has been marked as a duplicate of bug 73563 ***