Make -DUSE_REPL=On default #360
Conversation
|
Will test this locally soon. |
| @@ -43,7 +43,7 @@ if( CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR ) | |||
| endif() | |||
|
|
|||
| option(USE_CLING "Use Cling as backend" OFF) | |||
| option(USE_REPL "Use clang-repl as backend" OFF) | |||
| option(USE_REPL "Use clang-repl as backend" ON) | |||
There was a problem hiding this comment.
As the block below points out the following
if (USE_CLING AND USE_REPL)
message(FATAL_ERROR "We can only use Cling (USE_CLING=On) or Repl (USE_REPL=On), but not both of them.")
endif()
What can be done if we are using REPL as default .... is to make cling and REPL mutually exclusive. Which means we only use 1 of these by default.
- REPL by default , cling off by default
- And if cling is made on .... you can
set(USE_CLING ON)andunset(USE_REPL)or anything related in cmake.
So that if we encounter something like (the case you've not addressed yet)
CppInterOp/.github/workflows/ci.yml
Lines 873 to 874 in f794dee
We can just use USE_CLING and we don't need to explicitly set REPL to off.
There was a problem hiding this comment.
So we just end up with either the default case (which is supplying nothing, understood that repl is on and cling is off) or we just use cling=ON (understood repl would be off)
And if both if on by some chance we have the fatal error message.
|
clang-tidy review says "All clean, LGTM! 👍" |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #360 +/- ##
=======================================
Coverage 70.62% 70.62%
=======================================
Files 9 9
Lines 3500 3500
=======================================
Hits 2472 2472
Misses 1028 1028
|
|
clang-tidy review says "All clean, LGTM! 👍" |
|
@faze-geek Once you get this working you will also need to update the documentation to match this change. This includes both within the docs folder as well as the README.md . |
I have tested the pull request locally now. Once you guys are satisfied with the changes, I'll make the documentation change quick. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
faze-geek
force-pushed
the
change_cmake
branch
from
cafed0d to
9432bb5
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
| @@ -1580,7 +1578,6 @@ jobs: | |||
| else | |||
| emcmake cmake -DCMAKE_BUILD_TYPE=${{ env.BUILD_TYPE }} \ | |||
| -DUSE_CLING=OFF \ | |||
| -DUSE_REPL=ON \ | |||
There was a problem hiding this comment.
please make the similar changes in this file too
There was a problem hiding this comment.
I suggest he goes through all workflows in the repo.
There was a problem hiding this comment.
Thanks for letting me know.
| @@ -1191,7 +1191,6 @@ jobs: | |||
| else | |||
| cmake -DCMAKE_BUILD_TYPE=${{ env.BUILD_TYPE }} \ | |||
| -DUSE_CLING=OFF \ | |||
There was a problem hiding this comment.
@faze-geek If use_repl has been made the default then we should be able to remove all the uses of use_cling=off, since it should be off by default.
|
@faze-geek I just tried to rebase your PR, but it says it cannot be due to conflicts. Can you fix these conflicts and then rebase? |
|
clang-tidy review says "All clean, LGTM! 👍" |
Fixes #255