-
Notifications
You must be signed in to change notification settings - Fork 143
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Temporary revert cmake #176
Conversation
As long as cmake doesn't generate suitable deb packages, we need to switch back :(
6beb6b6
to
5bab975
Compare
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). EDIT : When you are used to CI tests, you automatically ask yourself 'does this change effect the CI stuff". |
Did you noticed that the whole CI stuff based on cmake? |
Den 19 juli 2020 13:35 skrev Stefan Wahren <[email protected]>:
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 GitHub<#176 (comment)>, or unsubscribe<https://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
|
As long as cmake doesn't generate suitable deb packages (see #174 ), we need to
switch back :(