Skip to content
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

fix(plugin-consortium-manual): drop repo constructor arg #1199 #1201

Merged
merged 4 commits into from
Aug 13, 2021

Conversation

petermetz
Copy link
Member

Removes the non-serializable consortiumRepo argument from the
constructor of the consortium plugin manual class's options.

Refactors the constructor and the internals of the plugin to initialize
the consortium repo from the consortium database at runtime
instead of expecting it passed in via the constructor.
Refactors the internal code previously using the options.consoritumRepo
object to use this.repo instead which is what gets initalized in
the constructor as explained above.

All this leads to equivalent functionality but less boilerplate and now
thanks to this the plugin can be (should be - more tests needed)
initialized by the API server purely based on the static configuration
file when necessary.

Fixes #1199

Signed-off-by: Peter Somogyvari peter.somogyvari@accenture.com

@codecov-commenter
Copy link

codecov-commenter commented Aug 10, 2021

Codecov Report

Merging #1201 (e40ac4e) into main (22eac9a) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1201   +/-   ##
=======================================
  Coverage   71.95%   71.96%           
=======================================
  Files         270      270           
  Lines        9431     9433    +2     
  Branches     1113     1113           
=======================================
+ Hits         6786     6788    +2     
  Misses       2072     2072           
  Partials      573      573           
Impacted Files Coverage Δ
...al/src/main/typescript/plugin-consortium-manual.ts 93.27% <100.00%> (+0.11%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 22eac9a...e40ac4e. Read the comment docs.

Copy link
Contributor

@jonathan-m-hamilton jonathan-m-hamilton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

…1199

Removes the non-serializable consortiumRepo argument from the
constructor of the consortium plugin manual class's options.

Refactors the constructor and the internals of the plugin  to initialize
the consortium repo from the consortium database at runtime
instead of expecting it passed in via the constructor.
Refactors the internal code previously using the options.consoritumRepo
object to use this.repo instead which is what gets initalized in
the constructor as explained above.

All this leads to equivalent functionality but less boilerplate and now
thanks to this the plugin can be (should be - more tests needed)
initialized by the API server purely based on the static configuration
file when necessary.

Fixes hyperledger#1199

Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
@petermetz petermetz merged commit 7b424d4 into hyperledger:main Aug 13, 2021
@petermetz petermetz deleted the fix-1199 branch August 13, 2021 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fix(plugin-consortium-manual): drop repo constructor arg
4 participants