Treat warnings as errors in build of cppjit - #45
Conversation
aaronj0
left a comment
There was a problem hiding this comment.
LGTM, thanks. Can you check if there is anything else thats stale in this MakeFile?
|
@aaronj0 I cannot see anything taking a quick look at the ci. If you update the reusable action here https://github.com/compiler-research/ci-workflows/blob/main/actions/build-and-test-cppjit/action.yml to do the cmake build with To match Clad, xeus-cpp, CppInterOp I think it would make sense to remove the handwritten makefile and replace it with a cmake file which the main cmake can call, but that is just personal choice. You could also change it so the tests are built with You will need to merge this PR if happy with it, since I don't have permissions to do anything in this repo. |
That sounds like a good idea, but we should probably first inject the flag in the CMakeLists.txt and test MakeFile here and see what fails. We can then address the remaining issues as a part of this PR, before making that default in the ci-workflows yml.
Regarding moving to a CMake file, that sounds like a good idea, but perhaps the simplicity of this MakeFile is desired :) |
|
I have added |
Sounds good, thank you! |
If you look at the nightly builds (here for example https://github.com/compiler-research/cppjit/actions/runs/32926841974/job/98051372290#step:8:695), you'll see that the
single_moduleis labelled as obsolete, so this PR removes the flag.