search

sogno-platform / dpsim

Real-time power system simulator including powerflow, (dynamic) phasors and EMT
https://sogno.energy/dpsim/
Mozilla Public License 2.0
67 stars 49 forks source link

Use clang-format to format the whole codebase #278

Closed stv0g closed 7 months ago

stv0g commented 7 months ago

Following up on the clang-format discussion, I now have drafted a PR to reformat the whole code base using the clang-format tool with its LLVM preset. Please have a look at the LLVM coding standards for a description of this style.

This PR does:

This PR does NOT:

The idea I have is that we are tolerant towards violate the LLVM style for any commits as we want to keep the barrier for new contributors as low as possible. We dont want to drive them away because of failing nit-pick CI checks.

Hence, we only reformat the entire code-base from time to time by hand.

And of course:

I invite all regular contributors to install the clang-format VSCode extension, so your changes are directly properly formatted when a file is saved.

(Btw. this also speeds up our own coding a lot as you will not have to worry ever again about code formatting. Just write as you like, save the file and everything is tidy :D)

sonarcloud[bot] commented 7 months ago

Quality Gate Failed Quality Gate failed

Failed conditions
E Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

idea Catch issues before they fail your Quality Gate with our IDE extension SonarLint SonarLint

stv0g commented 7 months ago

I think we will need to ignore the SonarCloud check in this instance as its merely a false positive. As the reformatting is touching almost all the code, it treats these changes as new code and mandates higher code quality standards.

And the Windows build appears to be broken for other PRs as well?

m-mirz commented 7 months ago

@stv0g it looks fine except for the Windows problem. The CI on the master does not seem to have this error related to spdlog: https://github.com/sogno-platform/dpsim/actions/runs/8049125560/job/21982389736?pr=278#step:3:867

leonardocarreras commented 7 months ago

Well, looks like @stv0g would probably be right, the last master was compiled with Current runner version: '2.311.0' while this one from the current PR is with Current runner version: '2.313.0' ... Among other things, cmake and lots of VS components were updated between those two versions...

Looks like we need to update (finally) spdlog or add more warning supressing declarations in the CmakeList https://github.com/sogno-platform/dpsim/blob/e0ae9d97db67b9554f019fc45dafa76f59a4da9e/CMakeLists.txt#L55-L73

Apparently are the following... define _SILENCE_STDEXT_ARR_ITERS_DEPRECATION_WARNING or _SILENCE_ALL_MS_EXT_DEPRECATION_WARNINGS as described in the step mentioned by @m-mirz

@stv0g it looks fine except for the Windows problem. The CI on the master does not seem to have this error related to spdlog: https://github.com/sogno-platform/dpsim/actions/runs/8049125560/job/21982389736?pr=278#step:3:867

I guess that is something to be solved in a different issue and merged before this PR, or not?

For Sonar, I agree that the code needs to be fixed at a different PR, not here.

m-mirz commented 7 months ago

@leonardocarreras @stv0g If the Windows build is broken anyway, we can also merge this PR first and solve this issue in a separate PR later ;-)