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

Move agency_comm to separate crate #193

Merged
merged 27 commits into from
Nov 13, 2020
Merged

Conversation

mirgee
Copy link
Contributor

@mirgee mirgee commented Nov 6, 2020

Signed-off-by: Miroslav Kovar miroslavkovar@protonmail.com

@mirgee mirgee force-pushed the refactor/separate-agency-comm branch from a81d012 to c4aeeb2 Compare November 6, 2020 17:11
@codecov-io
Copy link

codecov-io commented Nov 6, 2020

Codecov Report

Merging #193 (7f2c53d) into master (a58d5dc) will increase coverage by 3.32%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #193      +/-   ##
==========================================
+ Coverage   57.47%   60.79%   +3.32%     
==========================================
  Files         156      147       -9     
  Lines       23907    19648    -4259     
  Branches     6217     4322    -1895     
==========================================
- Hits        13740    11945    -1795     
+ Misses       6219     4255    -1964     
+ Partials     3948     3448     -500     
Flag Coverage Δ
integration 35.45% <22.28%> (+0.27%) ⬆️
unittests 53.49% <28.49%> (+2.79%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
agency_client/src/get_message.rs 85.71% <ø> (ø)
agency_client/src/payload.rs 0.00% <0.00%> (ø)
libvcx/src/api/connection.rs 39.87% <ø> (ø)
libvcx/src/api/credential.rs 33.56% <ø> (ø)
libvcx/src/api/disclosed_proof.rs 41.04% <ø> (ø)
libvcx/src/api/filters.rs 63.15% <ø> (ø)
libvcx/src/api/issuer_credential.rs 36.88% <ø> (ø)
libvcx/src/aries/handlers/connection/agent_info.rs 58.09% <0.00%> (-2.91%) ⬇️
libvcx/src/aries/handlers/connection/connection.rs 66.40% <ø> (ø)
libvcx/src/aries/messages/a2a/message_type.rs 77.41% <ø> (ø)
... and 67 more

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 a58d5dc...7f2c53d. Read the comment docs.

Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
@mirgee mirgee force-pushed the refactor/separate-agency-comm branch from c4aeeb2 to 1189320 Compare November 10, 2020 12:42
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
@mirgee mirgee force-pushed the refactor/separate-agency-comm branch 4 times, most recently from 67650b3 to 101e9c6 Compare November 11, 2020 10:26
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
@mirgee mirgee force-pushed the refactor/separate-agency-comm branch from 101e9c6 to c70e291 Compare November 11, 2020 10:30
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
@mirgee mirgee force-pushed the refactor/separate-agency-comm branch from 08de167 to b50905e Compare November 11, 2020 16:15
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
@Patrik-Stas
Copy link
Contributor

  • cargo.toml has extra dependencies (tokio-threadpool, libloading, ... probably more)
  • tests should be adjust not to work with with aries. for example we have test_update_agency_messages which first establishes aries connection - it was convenient, but it is not necessary to test the integration agency_comm/agency. We can create agent_connection in agency using the agency_comm interface, and we know how a message should be sent to this agent_connection.
  • I see you had to move bunch of stuff from utils aries-vcx module. I think little bit of duplication can be alright, sometimes its better than to overengineer and make module out of everything. The problem I see with this is that these functions are causing VcxError to be brough in, which seem unnecessary. I think we should return something like AgencyError from agency, and then let consumer of agency_comm module to deal with those errors (in case of vcx-aries as consumer of agency_comm, perhaps convert these errors)
  • Ultimately it would be good to get rid of agency_settings.rs so that a consumer of could create multiple agency_comm client instances.
  • How about renaming agency_comm to agency_client?

Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
@mirgee mirgee force-pushed the refactor/separate-agency-comm branch from 13b5707 to 5b711ee Compare November 12, 2020 13:34
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
@mirgee mirgee force-pushed the refactor/separate-agency-comm branch from 372d44e to f2f0b53 Compare November 12, 2020 15:43
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
@mirgee mirgee changed the title WIP: Move agency_comm to separate crate Move agency_comm to separate crate Nov 12, 2020
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
@mirgee mirgee force-pushed the refactor/separate-agency-comm branch from 75cc792 to 65b97bd Compare November 13, 2020 09:47
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
@mirgee mirgee force-pushed the refactor/separate-agency-comm branch from b744435 to 809f9dc Compare November 13, 2020 10:08
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
@Patrik-Stas
Copy link
Contributor

@mirgee we are on a good course with this, great job!

@Patrik-Stas Patrik-Stas merged commit f06ba4c into master Nov 13, 2020
@Patrik-Stas Patrik-Stas deleted the refactor/separate-agency-comm branch November 13, 2020 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants