skypjack / uvw

Header-only, event based, tiny and easy to use libuv wrapper in modern C++ - now available as also shared/static library!
MIT License
1.82k stars 207 forks source link

Check for GCC/Clang compiler warnings in Ubuntu GitHub Actions CI #304

Closed aloisklink closed 11 months ago

aloisklink commented 1 year ago

Add a CMakePresets.json file that contains ci-ubuntu preset. This preset contains most of the recommended compiler warnings from the cmake-init project, with the following changes:

I've also set the "CMAKE_CXX_FLAGS_DEBUG": "-Werror" variable when running on ubuntu in GitHub Actions. This will make the ubuntu build fail whenever any warnings are seen in the code, so you'll instantly spot it whenever somebody tries to add code that has warnings to this project. This should prevent issues like https://github.com/skypjack/uvw/issues/294 from happening in the future.


~I've left this PR as a draft, since the CI currently fails, but after the following PRs are merged, this CI job should work~ All of these PRs have now been merged, so CI passes:

skypjack commented 1 year ago

-Dlibuv_buildtests=OFF is gone during the rework. Am I wrong?

aloisklink commented 1 year ago

-Dlibuv_buildtests=OFF is gone during the rework. Am I wrong?

I've added it to the dev-mode CMake preset:

https://github.com/skypjack/uvw/blob/8a23e5808635b17e80f887482ec5f5f8cf33d3c6/CMakePresets.json#L26-L29

It will make it easier for us developers to run cmake --preset dev-mode locally, and in the future, we can add a ci-windows/ci-darwin presets for doing the Windows/MacOS CI runs, that also inherit from the dev-mode preset, so we'll only have to modify everything in a single file.

The only problem is that is cmake-presets are only supported in CMake 3.19 or later, but since we're only using them for CI/development (they won't be used by end-users), this isn't too big of an issue for us!

aloisklink commented 11 months ago

I've git rebase-d onto https://github.com/skypjack/uvw/commit/683883b08a6fdf19edc48d99ac1b901924a574cd on the main branch, since all the PRs have been merged and all the GCC/Clang compiler warnings have been fixed!

skypjack commented 11 months ago

Before merging, is this the last PR in the row or is there something else to address? Doing this work on warnings over the holidays made it a bit tricky to follow, sorry. 🙂

aloisklink commented 11 months ago

Before merging, is this the last PR in the row or is there something else to address?

This is the last PR in the row. Every other PR I made was about fixing warnings. This PR is just here to make big red cross in GitHub Actions if any new code is added that has warnings in it.

I considered doing something similar for Windows/MacOS, but I don't have easy access to a Windows/MacOS machine, so I guess that can wait until a Windows/MacOS user complains about warnings.

I do also want to make a PR fixing the https://skypjack.github.io/uvw/ website (currently a lot of links don't work, since they point to https://skypjack.github.io/uvw/classuvw_1_1emitter.html, but the file is actually called https://skypjack.github.io/uvw/classuvw_1_1Emitter.html (e.g. with a capital E not e)), but that's completely unrelated to this compiler warnings series of PRs.

Doing this work on warnings over the holidays made it a bit tricky to follow, sorry. 🙂

And no worries :) Thanks for reviewing stuff on your holiday! To be honest, I wouldn't have minded waiting. After all, since these PRs were mostly just refactoring PRs, there's probably no point in making a uvw release just for this :smile: