Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Move 2 MSVC warnings outside if ENABLE_WARNINGS #7

Closed
HackingPheasant opened this issue Jul 17, 2021 · 2 comments
Closed

Move 2 MSVC warnings outside if ENABLE_WARNINGS #7

HackingPheasant opened this issue Jul 17, 2021 · 2 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@HackingPheasant
Copy link
Owner

HackingPheasant commented Jul 17, 2021 •
edited
Loading

manga-manager/cmake/CompilerWarnings.cmake

Lines 111 to 112 in 08cb83a

"/permissive-" # standards conformance mode for MSVC compiler.
"NOMINMAX" # Stop windows.h min/max macros conflicting with C++ Standard Library

Best to move these out of the ENABLE_WARNINGS section code and enable them unconditionally.

Edit: Move NOMINMAX to an if(WINDOWS) block
@HackingPheasant HackingPheasant added enhancement New feature or request good first issue Good for newcomers labels Jul 17, 2021
@hermantrym
Copy link

I think it is necessary to define the same macros as in the if and else if branches. Even if you don’t use another compiler than Visual Studio, gcc or clang today, it would be a shame to halt the compilation on another compiler just because you didn’t define the macros for it.

@HackingPheasant
Copy link
Owner Author

Yeah, good point I'll be best moving the NOMINMAX into something like an if(WINDOWS) block, instead of it being added into each compiler (or at the moment only being applied to MSVC).

HackingPheasant added a commit that referenced this issue Nov 15, 2021
@HackingPheasant
everything: Massive (overdue) cleanup
6493881 
- Update 3rd party libraries to the newest version (Closes #6)
- Removed Mangadex code since I will need to redo all of it anyways
- Move Windows NOMINMAX define out of MSVC compiler to an if(Windows)
  conditional (Closes #7)
- Move MSVC /permissive- flag outside of the if(ENABLE_WARNINGS)
  conditional
  (Closes #7)
- Merge Clang and AppleClang flags (again)
- Clean-up top level CMakeLists.txt by moving the changes to appropriate
  .cmake files
- Drop generator from CMakePresets.json now that its not required by
  default.
- Update .gitignore
- Restructure how I handle handle source files. The time to do this sort
  of thing is now, figuring out what works best for me and the project is
  when its infancy and I am the sole contributor.

We do include the beginning skeleton of the new window.[cpp/h] files
since it was easier then more mucking about in the cmake files. ;)
HackingPheasant added a commit that referenced this issue Sep 2, 2024
@HackingPheasant
everything: Massive (overdue) cleanup
88a5621 
- Update 3rd party libraries to the newest version (Closes #6)
- Removed Mangadex code since I will need to redo all of it anyways
- Move Windows NOMINMAX define out of MSVC compiler to an if(Windows)
  conditional (Closes #7)
- Move MSVC /permissive- flag outside of the if(ENABLE_WARNINGS)
  conditional
  (Closes #7)
- Merge Clang and AppleClang flags (again)
- Clean-up top level CMakeLists.txt by moving the changes to appropriate
  .cmake files
- Drop generator from CMakePresets.json now that its not required by
  default.
- Update .gitignore
- Restructure how I handle handle source files. The time to do this sort
  of thing is now, figuring out what works best for me and the project is
  when its infancy and I am the sole contributor.

We do include the beginning skeleton of the new window.[cpp/h] files
since it was easier then more mucking about in the cmake files. ;)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
Inital 1.0 Release
Status: Done
Milestone
No milestone
Development

No branches or pull requests
2 participants
@HackingPheasant @hermantrym