khrynczenko / RadioStream

Simple, fast and light-weight internet radio player for Windows and Linux
MIT License
19 stars 3 forks source link

Enforce apropariate amount of warnings and treat them as errors #73

Closed khrynczenko closed 4 years ago

khrynczenko commented 4 years ago

I think that adding compiler fags in cmake would be a good thing.
Also it believe that turning warnings into errors is the only way. Otherwise neglect creeps in and in a moment there are hundreds of unresolved warnings.

For MSVC it would encompass:

For GCC this would encompass:

It this is too much we could start with just first two for MSVC and GCC. Also CI for MSVC would be much needed in case someone works with GCC because we would need to check their changes against MSVC.

khrynczenko commented 4 years ago

Because dependencies are added in cmake with add_subdirectory, added compiler option propagate to them as well. This is unfortunate because /WX or -Werror will error not only on problems in RadioStream but also in POCO, nana etc.
Maybe we could change CMakeLists.txt so that dependencies are added with ExternalProject_Add command instead. This would allow us to set seperate options for us and them.

khrynczenko commented 4 years ago

So the flags were added in #74. Unfortunately the header files of the external project still gave errors because MSVC doesn't provide a way to specify inlude directories that are external (like gcc isystem <path>). Only way to do that for now is to surround external includes with

#pragma warning (push, 0)
// external includes
#pragma warning (pop)

Anyway it works and is resolved for now.