refactor: Bump min json ver, make json keys static#2968
refactor: Bump min json ver, make json keys static#2968paulgessinger wants to merge 12 commits intoacts-project:mainfrom
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2968 +/- ##
==========================================
+ Coverage 47.26% 48.86% +1.59%
==========================================
Files 510 493 -17
Lines 30096 29058 -1038
Branches 14605 13798 -807
==========================================
- Hits 14226 14200 -26
+ Misses 5378 4962 -416
+ Partials 10492 9896 -596 ☔ View full report in Codecov by Sentry. |
02b7960 to
dbc3c50
Compare
dbc3c50 to
84afa5f
Compare
|
|
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughUpdated, the nlohmann_json dependency version and checksum are. Refactored, JSON key storage is, from struct members to static constexpr string_views in a namespace. Adjusted, all usages of these keys in material and volume JSON converters are. No logic or control flow changed, hmmm. Changes
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
still relevant @paulgessinger ? |
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Plugins/Json/src/MaterialJsonConverter.cpp (1)
457-469: Guard the lookup, safer it becomes.
jMaterial[Acts::JsonKey::kMaterialMap]used withoutcontains()check; when key absent, default-constructs null and still compares, undefined logic it invites. Prefer explicit guard:-if (jMaterial[Acts::JsonKey::kMaterialMap] == false) { +if (!jMaterial.contains(Acts::JsonKey::kMaterialMap) || + jMaterial[Acts::JsonKey::kMaterialMap] == false) { return; }Also applies to: 590-598
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
CMakeLists.txt(1 hunks)Plugins/Json/include/Acts/Plugins/Json/GeometryJsonKeys.hpp(1 hunks)Plugins/Json/src/MaterialJsonConverter.cpp(13 hunks)Plugins/Json/src/VolumeJsonConverter.cpp(1 hunks)cmake/ActsExternSources.cmake(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (150)
- GitHub Check: CI Bridge / lcg_107: [alma9, clang19]
- GitHub Check: CI Bridge / build_gnn_tensorrt
- GitHub Check: CI Bridge / lcg_107: [alma9, gcc14]
- GitHub Check: CI Bridge / lcg_107: [alma9, gcc13]
- GitHub Check: CI Bridge / lcg_106a: [alma9, clang16]
- GitHub Check: CI Bridge / lcg_106a: [alma9, gcc14]
- GitHub Check: CI Bridge / lcg_106a: [alma9, gcc13]
- GitHub Check: CI Bridge / build_exatrkx_cpu
- GitHub Check: CI Bridge / clang_tidy
- GitHub Check: CI Bridge / lcg_105: [alma9, gcc13]
- GitHub Check: CI Bridge / linux_ubuntu_2204
- GitHub Check: CI Bridge / lcg_105: [alma9, clang16]
- GitHub Check: CI Bridge / build_exatrkx
- GitHub Check: CI Bridge / build_linux_ubuntu
- GitHub Check: CI Bridge / linux_ubuntu_2404_clang19
- GitHub Check: CI Bridge / lcg_107: [alma9, clang19]
- GitHub Check: CI Bridge / build_gnn_tensorrt
- GitHub Check: CI Bridge / lcg_107: [alma9, gcc14]
- GitHub Check: CI Bridge / lcg_107: [alma9, gcc13]
- GitHub Check: CI Bridge / lcg_106a: [alma9, clang16]
- GitHub Check: CI Bridge / lcg_106a: [alma9, gcc14]
- GitHub Check: CI Bridge / lcg_106a: [alma9, gcc13]
- GitHub Check: CI Bridge / build_exatrkx_cpu
- GitHub Check: CI Bridge / clang_tidy
- GitHub Check: CI Bridge / lcg_105: [alma9, gcc13]
- GitHub Check: CI Bridge / linux_ubuntu_2204
- GitHub Check: CI Bridge / lcg_105: [alma9, clang16]
- GitHub Check: CI Bridge / build_exatrkx
- GitHub Check: CI Bridge / build_linux_ubuntu
- GitHub Check: CI Bridge / linux_ubuntu_2404_clang19
- GitHub Check: CI Bridge / lcg_107: [alma9, clang19]
- GitHub Check: CI Bridge / build_gnn_tensorrt
- GitHub Check: CI Bridge / lcg_107: [alma9, gcc14]
- GitHub Check: CI Bridge / lcg_107: [alma9, gcc13]
- GitHub Check: CI Bridge / lcg_106a: [alma9, clang16]
- GitHub Check: CI Bridge / lcg_106a: [alma9, gcc14]
- GitHub Check: CI Bridge / lcg_106a: [alma9, gcc13]
- GitHub Check: CI Bridge / build_exatrkx_cpu
- GitHub Check: CI Bridge / clang_tidy
- GitHub Check: CI Bridge / lcg_105: [alma9, gcc13]
- GitHub Check: CI Bridge / linux_ubuntu_2204
- GitHub Check: CI Bridge / lcg_105: [alma9, clang16]
- GitHub Check: CI Bridge / build_exatrkx
- GitHub Check: CI Bridge / build_linux_ubuntu
- GitHub Check: CI Bridge / linux_ubuntu_2404_clang19
- GitHub Check: CI Bridge / lcg_107: [alma9, clang19]
- GitHub Check: CI Bridge / build_gnn_tensorrt
- GitHub Check: CI Bridge / lcg_107: [alma9, gcc14]
- GitHub Check: CI Bridge / lcg_107: [alma9, gcc13]
- GitHub Check: CI Bridge / lcg_106a: [alma9, clang16]
- GitHub Check: CI Bridge / lcg_106a: [alma9, gcc14]
- GitHub Check: CI Bridge / lcg_106a: [alma9, gcc13]
- GitHub Check: CI Bridge / build_exatrkx_cpu
- GitHub Check: CI Bridge / clang_tidy
- GitHub Check: CI Bridge / lcg_105: [alma9, gcc13]
- GitHub Check: CI Bridge / linux_ubuntu_2204
- GitHub Check: CI Bridge / lcg_105: [alma9, clang16]
- GitHub Check: CI Bridge / build_exatrkx
- GitHub Check: CI Bridge / build_linux_ubuntu
- GitHub Check: CI Bridge / linux_ubuntu_2404_clang19
- GitHub Check: CI Bridge / lcg_107: [alma9, clang19]
- GitHub Check: CI Bridge / build_gnn_tensorrt
- GitHub Check: CI Bridge / lcg_107: [alma9, gcc14]
- GitHub Check: CI Bridge / lcg_107: [alma9, gcc13]
- GitHub Check: CI Bridge / lcg_106a: [alma9, clang16]
- GitHub Check: CI Bridge / lcg_106a: [alma9, gcc14]
- GitHub Check: CI Bridge / lcg_106a: [alma9, gcc13]
- GitHub Check: CI Bridge / build_exatrkx_cpu
- GitHub Check: CI Bridge / clang_tidy
- GitHub Check: CI Bridge / lcg_105: [alma9, gcc13]
- GitHub Check: CI Bridge / linux_ubuntu_2204
- GitHub Check: CI Bridge / lcg_105: [alma9, clang16]
- GitHub Check: CI Bridge / build_exatrkx
- GitHub Check: CI Bridge / build_linux_ubuntu
- GitHub Check: CI Bridge / linux_ubuntu_2404_clang19
- GitHub Check: CI Bridge / lcg_107: [alma9, clang19]
- GitHub Check: CI Bridge / build_gnn_tensorrt
- GitHub Check: CI Bridge / lcg_107: [alma9, gcc14]
- GitHub Check: CI Bridge / lcg_107: [alma9, gcc13]
- GitHub Check: CI Bridge / lcg_106a: [alma9, clang16]
- GitHub Check: CI Bridge / lcg_106a: [alma9, gcc14]
- GitHub Check: CI Bridge / lcg_106a: [alma9, gcc13]
- GitHub Check: CI Bridge / build_exatrkx_cpu
- GitHub Check: CI Bridge / clang_tidy
- GitHub Check: CI Bridge / lcg_105: [alma9, gcc13]
- GitHub Check: CI Bridge / linux_ubuntu_2204
- GitHub Check: CI Bridge / lcg_105: [alma9, clang16]
- GitHub Check: CI Bridge / build_exatrkx
- GitHub Check: CI Bridge / build_linux_ubuntu
- GitHub Check: CI Bridge / linux_ubuntu_2404_clang19
- GitHub Check: CI Bridge / lcg_107: [alma9, clang19]
- GitHub Check: CI Bridge / build_gnn_tensorrt
- GitHub Check: CI Bridge / lcg_107: [alma9, gcc14]
- GitHub Check: CI Bridge / lcg_107: [alma9, gcc13]
- GitHub Check: CI Bridge / lcg_106a: [alma9, clang16]
- GitHub Check: CI Bridge / lcg_106a: [alma9, gcc14]
- GitHub Check: CI Bridge / lcg_106a: [alma9, gcc13]
- GitHub Check: CI Bridge / build_exatrkx_cpu
- GitHub Check: CI Bridge / clang_tidy
- GitHub Check: CI Bridge / lcg_105: [alma9, gcc13]
- GitHub Check: CI Bridge / linux_ubuntu_2204
- GitHub Check: CI Bridge / lcg_105: [alma9, clang16]
- GitHub Check: CI Bridge / build_exatrkx
- GitHub Check: CI Bridge / build_linux_ubuntu
- GitHub Check: CI Bridge / linux_ubuntu_2404_clang19
- GitHub Check: CI Bridge / lcg_107: [alma9, clang19]
- GitHub Check: CI Bridge / build_gnn_tensorrt
- GitHub Check: CI Bridge / lcg_107: [alma9, gcc14]
- GitHub Check: CI Bridge / lcg_107: [alma9, gcc13]
- GitHub Check: CI Bridge / lcg_106a: [alma9, clang16]
- GitHub Check: CI Bridge / lcg_106a: [alma9, gcc14]
- GitHub Check: CI Bridge / lcg_106a: [alma9, gcc13]
- GitHub Check: CI Bridge / build_exatrkx_cpu
- GitHub Check: CI Bridge / clang_tidy
- GitHub Check: CI Bridge / lcg_105: [alma9, gcc13]
- GitHub Check: CI Bridge / linux_ubuntu_2204
- GitHub Check: CI Bridge / lcg_105: [alma9, clang16]
- GitHub Check: CI Bridge / build_exatrkx
- GitHub Check: CI Bridge / build_linux_ubuntu
- GitHub Check: CI Bridge / linux_ubuntu_2404_clang19
- GitHub Check: CI Bridge / lcg_107: [alma9, clang19]
- GitHub Check: CI Bridge / build_gnn_tensorrt
- GitHub Check: CI Bridge / lcg_107: [alma9, gcc14]
- GitHub Check: CI Bridge / lcg_107: [alma9, gcc13]
- GitHub Check: CI Bridge / lcg_106a: [alma9, clang16]
- GitHub Check: CI Bridge / lcg_106a: [alma9, gcc14]
- GitHub Check: CI Bridge / lcg_106a: [alma9, gcc13]
- GitHub Check: CI Bridge / build_exatrkx_cpu
- GitHub Check: CI Bridge / clang_tidy
- GitHub Check: CI Bridge / lcg_105: [alma9, gcc13]
- GitHub Check: CI Bridge / linux_ubuntu_2204
- GitHub Check: CI Bridge / lcg_105: [alma9, clang16]
- GitHub Check: CI Bridge / build_exatrkx
- GitHub Check: CI Bridge / build_linux_ubuntu
- GitHub Check: CI Bridge / linux_ubuntu_2404_clang19
- GitHub Check: CI Bridge / lcg_107: [alma9, clang19]
- GitHub Check: CI Bridge / build_gnn_tensorrt
- GitHub Check: CI Bridge / lcg_107: [alma9, gcc14]
- GitHub Check: CI Bridge / lcg_107: [alma9, gcc13]
- GitHub Check: CI Bridge / lcg_106a: [alma9, clang16]
- GitHub Check: CI Bridge / lcg_106a: [alma9, gcc14]
- GitHub Check: CI Bridge / lcg_106a: [alma9, gcc13]
- GitHub Check: CI Bridge / build_exatrkx_cpu
- GitHub Check: CI Bridge / clang_tidy
- GitHub Check: CI Bridge / lcg_105: [alma9, gcc13]
- GitHub Check: CI Bridge / linux_ubuntu_2204
- GitHub Check: CI Bridge / lcg_105: [alma9, clang16]
- GitHub Check: CI Bridge / build_exatrkx
- GitHub Check: CI Bridge / build_linux_ubuntu
- GitHub Check: CI Bridge / linux_ubuntu_2404_clang19
🔇 Additional comments (5)
CMakeLists.txt (1)
280-280: Bumped the force, you have; environment update, verify must.Version 3.11.2 set, yet CI speaks of LCG105 holding 3.10.5. Build shall fail, hmm? Ensure container or module upgraded before merge, you should.
cmake/ActsExternSources.cmake (1)
50-52: Checksum matches the new reality, confirm we must.Digest updated; good this seems. Still, run
sha256sumon the tarball to be certain, you could. A corrupted archive, avoid we must.Plugins/Json/src/VolumeJsonConverter.cpp (1)
19-25: Static key usage, elegant this is.Compile-time constant over dynamic struct, faster and safer. Change clean, nothing else to note.
Plugins/Json/src/MaterialJsonConverter.cpp (1)
90-96: Static keys embraced, consistency brought.Refactor clear, performance neutral, readability improved. Approve, I do.
Plugins/Json/include/Acts/Plugins/Json/GeometryJsonKeys.hpp (1)
11-33: Constexpr string_view, strong with the force it is.Memory saved, initialization order issues vanished. Good change.
|
still relevant @paulgessinger ? |





Summary by CodeRabbit
Refactor
Chores