Return-To-The-Roots / s25client

Return To The Roots (Settlers II(R) Clone)
http://www.rttr.info
GNU General Public License v2.0
470 stars 75 forks source link

Bump C++ standard version to C++17 #1618

Closed falbrechtskirchinger closed 11 months ago

falbrechtskirchinger commented 11 months ago

There're about 4 commits that aren't strictly about C++17 but clang-tidy fixes I noticed along the way, like removing // NOLINT entirely or replacing it with a more specific suppression. Also, some changes apply to checks in newer clang-tidy versions only, like the De Morgan's theorem one in RTTR_Assert.

The majority of changes stem from the Re-format after namespace concatenation commit. It might be easier to remove that commit via an interactive rebase, recreate it by running clang-format, and compare it against my branch.

To-do:

falbrechtskirchinger commented 11 months ago

I like it, especially the compact namespaces and the unary static-assert.

The majority of changes stem from the Re-format after namespace concatenation commit. It might be easier to remove that commit via an interactive rebase, recreate it by running clang-format, and compare it against my branch.

You can view the diff without whitespace changes which looks clean enough.

Huh, either never noticed that button on GH before, or forgot about it. Thanks!

We do have the C++ standard set to 14 in .clang-format though. Do we need to update that too? Does it change anything?

Makes sense to bump that as well.

I would love to keep functions containing only a return statement on a single line, but I guess that isn't (easily) possible (see the note). Have you checked anything related to that already?

Noticed that too but didn't look for a solution. I'll see what I can do.

falbrechtskirchinger commented 11 months ago

Bumping the standard in .clang-format had no impact on formatting.