Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #141 +/- ##
=======================================
Coverage 80.31% 80.31%
=======================================
Files 115 115
Lines 1910 1910
Branches 302 302
=======================================
Hits 1534 1534
Misses 247 247
Partials 129 129
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
aolivier23
left a comment
aolivier23
left a comment
There was a problem hiding this comment.
Thank you Kyle, looks good to me.
| ) # REFLEX_GENERATE_DICTIONARY doesn't work trivially without making a shared | ||
| # library | ||
| target_link_libraries(${FORM_DATA_PROD_LIB_NAME}) | ||
| target_link_libraries(${FORM_DATA_PROD_LIB_NAME} ROOT::RIO) |
There was a problem hiding this comment.
I was wondering whether the ROOT::RIO addition could be hidden behind a if(FORM_USE_ROOT_STORAGE)
There was a problem hiding this comment.
Sorry, Peter. Just merged this PR. I made the assumption that form_test_data_products corresponded to the dictionary library. I think I can do what you suggest...I'll create another PR.
There was a problem hiding this comment.
I think it's hidden in the next level up here:
phlex/test/form/CMakeLists.txt
Line 3 in 6838973
There was a problem hiding this comment.
Even if I'm right, we might still want finer-grained control of when FORM uses ROOT. Right now, I think our tests rely on having a ROOT back end.
There was a problem hiding this comment.
And be consistent with https://github.com/knoepfel/phlex/blob/0814aa71f34e4d3d53eb41c638fd84d83fcb90c6/test/form/data_products/CMakeLists.txt#L8
maybe we don't need either...
There was a problem hiding this comment.
Well, here's a fix that at least keeps the library with track_start from depending explicitly on ROOT unless there's a need. I understand that this code will unconditionally execute when FORM_USE_ROOT_STORAGE is true, but maybe that PR makes refactoring easier later on?
3 participants
This PR:
ROOT::RIOlibrary during the dictionary-generation stepsnprintfinstead ofsprintfwhen filling a string buffer (per apple-clang warning)