search

ihedvall / mdflib

Implementation of the ASAM MDF data file.
https://ihedvall.github.io/mdflib/
MIT License
57 stars 26 forks source link

Format code #4

Open Murmele opened 1 year ago

Murmele commented 1 year ago

format code according to Google standard: clang-format --style=Google -dump-config > .clangformat Add ci to check if code format is correctly

When I run the Google style with clang-format, it seems a lot of files get touched

ihedvall commented 1 year ago

It's better to wait with the formatting features until we have a stable code base.

Murmele commented 1 year ago

Ok I will let it open.

ihedvall commented 1 year ago
  1. I have tested the clang format. It is working for most C++ files but not for wxWidgets related C++ files.
  2. Should the the format file be named '.clang-format' or '.clangformat' ?
  3. Is it possible to separate the actual build from the format check by two yaml files?
  4. The bash script is somewhat complicated.
ihedvall commented 1 year ago

Is it possible to skip the cmake file formatting?

Murmele commented 1 year ago

I changed it to format only the source code and no cmake

Murmele commented 1 year ago

You can set in the settings of github that merge pull request only allowed when CI build is running through. I will check out why it failed

Murmele commented 1 year ago

I have to check why it crashes after formatting the code

ihedvall commented 1 year ago

I have added the clang format to the utillib project so the CLION uses its formatting rules. I notice that is changes the order of include files which most likely causing some build problems.

I can do the same on the mdflib project but this will generate a difficult merge.

Murmele commented 1 year ago

We can also turn off resorting includes? Why it is getting a difficult merge? If you are doing it on master it should bring additional issues?

ihedvall commented 1 year ago

Doing the clang format in the editor, then I can compile before pushing. This solves the initial checkin. Setting up the editor to automatic reformat files at save minimizes the risk of further clang format issues.

Do you want me to do the intial clang-format checkin on master?

Murmele commented 1 year ago

Doing the clang format in the editor, then I can compile before pushing. This solves the initial checkin. Setting up the editor to automatic reformat files at save minimizes the risk of further clang format issues.

Do you want me to do the intial clang-format checkin on master?

yes you can do. I don't have anything against. currently I don't have changed source code so I don't have to do any merge requests

ihedvall commented 1 year ago

The TestInstall fails in the build due to expat library missing. I'm lost in the cmake-nested-cmake. Do you have a solution of mdflib dependency of expat library?

Murmele commented 1 year ago

The TestInstall fails in the build due to expat library missing. I'm lost in the cmake-nested-cmake. Do you have a solution of mdflib dependency of expat library?

I will check out. I think in the Test we should not link to expat, because it is not directly needed. It should be linked in mdflib. I will check it out.

Murmele commented 1 year ago

there is not find_package() for expat anymore. So you would like to force to use the downloaded version?

Murmele commented 1 year ago

EXPAT_LIBRARIES gets also not populated