search

rscada / libmbus

Meter-bus library and utility programs
http://www.rscada.se/libmbus
BSD 3-Clause "New" or "Revised" License
217 stars 137 forks source link

Temporary revert cmake #176

Closed lategoodbye closed 3 years ago

lategoodbye commented 3 years ago

As long as cmake doesn't generate suitable deb packages (see #174 ), we need to switch back :(

fredrik-sk commented 3 years ago

Did this change pass the CI pipelines? No.

https://github.com/rscada/libmbus/actions/runs/174538963

The code might work (but I am not sure since I have not seen proof). I would assume that the file specifing how the automatic builds are executed should have been updated as well.

EDIT : When you are used to CI tests, you automatically ask yourself 'does this change effect the CI stuff".

lategoodbye commented 3 years ago

Did you noticed that the whole CI stuff based on cmake?

fredrik-sk commented 3 years ago

Den 19 juli 2020 13:35 skrev Stefan Wahren notifications@github.com:

Did you noticed that the whole CI stuff based on cmake?

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/rscada/libmbus/pull/176#issuecomment-660630263, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AP2YIODKZDCYJTMYDRFLJX3R4LLAXANCNFSM4PBF7KAQ.

I would say that of course should the CI-stuff match the current build system and tests.

So when switching to using configure + make, the commands for the CI stuff should be updated to use configure + make. Or call a script like build.sh if that is what we want to do from the CI tests. (I did not look at the your patch so I don't know the exact details).

In this particular case, you might have introduced the CI test when introducing CMake, but there is nothing that says that CI tests are not possible when using configure + make.

What commands did you use to build and test the code? You should replace the cmake stuff with these commands in the YAML file.

/F