Remove some deprecated features #1806
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1806 +/- ##
==========================================
- Coverage 45.45% 45.45% -0.01%
==========================================
Files 550 550
Lines 113139 113138 -1
==========================================
- Hits 51423 51422 -1
Misses 61716 61716
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
| @@ -353,7 +353,7 @@ if(NRN_ENABLE_INTERVIEWS) | |||
| get_target_property(IV_INCLUDE_DIR interviews INTERFACE_INCLUDE_DIRECTORIES) | |||
| else() | |||
| nrn_add_external_project(iv) | |||
| include_directories(external/iv/src/include) | |||
| include_directories(SYSTEM external/iv/src/include) | |||
There was a problem hiding this comment.
Just curious. What is the rationale. I see from cmake doc that
If the SYSTEM option is given, the compiler will be told the directories are meant as system include directories on some platforms. Signalling this setting might achieve effects such as the compiler skipping warnings, or these fixed-install system files not being considered in dependency calculations - see compiler docs.
Is this mostly for skipping warnings? Co-development of IV and nrn is rare, so not being considered in dependency calculations isn't a strong negative. The "some platforms" caveat makes it all foggy to me.
There was a problem hiding this comment.
I got a lot of warning from IV but I think we don't want to fix them ,right? as it is an external dependency, so we don't see the internal warning of IV. If you misuse the public API of IV inside nrn it will produce warning.
There was a problem hiding this comment.
Ok. That sounds right. I might someday work on that to make it -Wall clean, but it's low priority.
|
If you're finished with this. I'll merge. |
|
It's alright |
No description provided.