-
Notifications
You must be signed in to change notification settings - Fork 83
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Vdrtools & Modular Libs as feature flags #763
Conversation
Signed-off-by: gmulhearn <gmulhearn@proton.me>
Signed-off-by: gmulhearn <gmulhearn@proton.me>
Signed-off-by: gmulhearn <gmulhearn@proton.me>
Codecov Report
@@ Coverage Diff @@
## main #763 +/- ##
==========================================
+ Coverage 46.93% 54.19% +7.26%
==========================================
Files 381 381
Lines 32516 36850 +4334
Branches 7202 8307 +1105
==========================================
+ Hits 15260 19972 +4712
+ Misses 12491 10713 -1778
- Partials 4765 6165 +1400
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 121 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Signed-off-by: gmulhearn <gmulhearn@proton.me>
Signed-off-by: gmulhearn <gmulhearn@proton.me>
Signed-off-by: gmulhearn <gmulhearn@proton.me>
Signed-off-by: gmulhearn <gmulhearn@proton.me>
@Patrik-Stas requested for review a bit early bcus i'm too impatient for wait for the pipeline! I will double check that it has passed later... but it should be good |
Signed-off-by: Patrik <patrik.stas@absa.africa>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice job with feature documentation
@gmulhearn another great incremental step forward! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm okay with the changes considering the goal of the PR.
I think that in the future, though, we should strive towards refactoring all this test feature flag and mocks mess outside of aries-vcx (at least it's src
).
Since aries-vcx does not depend on libvdrtoola
, then it shouldn't even be used. We could maybe provide a vdrtools-based wallet et al in a separate crate that can be used by consumers.
@bobozaur just some clarification
aries-vcx does depend on libvdrtools, but the goal is to get rid of it - the feature flags are way to opt-in either The testing feature flags are orthogonal to the changes in this pr, though I don't disagree there can be better way to test |
* Add feature flags to opt-in vdrtools or modular libs implementation Signed-off-by: gmulhearn <gmulhearn@proton.me> Co-authored-by: George Mulhearn <gmulhearn@anonyome.com> Signed-off-by: Andy Waine <88730354+Andy-Waine@users.noreply.github.com>
This PR moves vdrtools & modular dependencies (indy-credx & indy-vdr) under their own separate feature flags.
The idea is that consumers who do not want to use vdrtools dependencies (e.g. using modular deps or their even own deps instead), can override the default flag and use their own
Profile
. This way the consumer does not have to pull in the heavy-weight deps of libvdrtools. Likewise formodular_libs
.I've kept
vdrtools
as the default feature foraries_vcx
, as it is most likely what is currently used the most. In the future we may think about changing thisIt is also enforced, that when testing with
test_utils
, bothvdrtools
andmodular_libs
must be enabled.Other Changes
LibIndyMocks
structure out of indy. There is now a global statuscodemock for the same functionality, however in the long term, we should remove thismodular_dependencies
tomodular_libs_tests
to better represent that it is just a testing feature flag.