refactor: Remove dependency on Boost::program_options from Algorithms/Writers/Readers #1610
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
October 20, 2022 19:22
benjaminhuth
added
Impact - Minor
Codecov Report
@@ Coverage Diff @@
## main #1610 +/- ##
=======================================
Coverage 48.66% 48.66%
=======================================
Files 381 381
Lines 20776 20776
Branches 9517 9517
=======================================
Hits 10110 10110
Misses 4095 4095
Partials 6571 6571 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
@benjaminhuth do you want feedback now already or not yet? |
|
If you have some general feedback, go ahead. I think feedback on implementation details should wait until this compiles |
...les/Detectors/ContextualDetector/include/ActsExamples/ContextualDetector/AlignedDetector.hpp
Outdated
Show resolved
Hide resolved
Examples/Detectors/DD4hepDetector/include/ActsExamples/DD4hepDetector/DD4hepDetector.hpp
Outdated
Show resolved
Hide resolved
Examples/Detectors/EmptyDetector/include/ActsExamples/EmptyDetector/EmptyDetector.hpp
Outdated
Show resolved
Hide resolved
Examples/Detectors/GenericDetector/include/ActsExamples/GenericDetector/GenericDetector.hpp
Outdated
Show resolved
Hide resolved
Examples/Detectors/TGeoDetector/include/ActsExamples/TGeoDetector/TGeoDetector.hpp
Outdated
Show resolved
Hide resolved
Examples/Run/Common/include/ActsExamples/Detector/IBaseDetector.hpp
Outdated
Show resolved
Hide resolved
|
I think I've cleaned this up, if the CI goes through could you have a look again @paulgessinger ? |
📊 Physics performance monitoring for f503cfeFull report VertexingIVF seededIVF truth smearedIVF truth estimatedCKFseededtruth smearedtruth estimatedAmbiguity resolutionseededTruth trackingTruth tracking |
|
@paulgessinger the CI is running through now :) |
andiwand
approved these changes
Oct 25, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This aims to be the first step into making the binaries optional to build by removing the dependencies on
Boost::program_optionsfrom the Algorithms/Writers/Readers.The second step would be to also unwind this from the Detectors, but this is a bit more complicated, and thus I would do this step by step.
A long term goal would be that only the Analysis Apps depend on Boost program options, so that just building the Examples does not need any Boost components that require binaries (since we also use
std::filesystemnow).