Workspace reorganization #998

bobozaur · 2023-09-26T10:31:16Z

This PR aims to address the ever growing number of crates we have in the workspace so that we make it easier for newcomers as well as us to navigate through the directory structure.

This top comment will be updated with the directory structure after it's debated enough and agreed upon. Combining existent crates is represented by + while creating a new directory where existent crates will be moved is represented by a nested bullet point list.

Directory structure:

.github
docs
agents
- rust
- node
aries_vcx
aries_vcx_core
did_doc
- did_doc_core
- did_doc_sov
did
- did_key
- did_parser
- did_peer
- did_resolver
  - did_resolver_core
  - did_resolver_sov
  - did_resolver_web
  - did_resolver_registry
public_key
aries_messages
- messages
- messages_macros
tools
- simple_message_relay
- wallet_migrator
legacy
- did_doc_legacy
- libvdrtools
- agency_client
shared_vcx
wrappers
- node
  - libvcx_core
  - node
  - vcx-napi-rs
- uniffi-aries-vcx

NOTE: The ci directory gets moved under .github in #1001 .

Questions:

Remove did_resolver_registry? It's unused, small and not really something I personally like due to being an unnecessary batteries included abstraction over something consumers should build from scratch.
ANSWER: used in DID Exchange Protocol #928 .
Do we still need agency_client? Could we remove it or should it just be moved to the legacy directory?
ANSWER: agency_client is still needed so we'll place it in the legacy directory.

codecov-commenter · 2023-09-26T10:56:12Z

Codecov Report

Merging #998 (583986f) into main (ac35fdd) will increase coverage by 0.02%.
The diff coverage is 0.00%.

❗ Current head 583986f differs from pull request most recent head b068a6d. Consider uploading reports for the commit b068a6d to get more accurate results

@@            Coverage Diff             @@
##             main     #998      +/-   ##
==========================================
+ Coverage   30.00%   30.03%   +0.02%     
==========================================
  Files         416      415       -1     
  Lines       26929    26918      -11     
  Branches     5244     5243       -1     
==========================================
+ Hits         8080     8084       +4     
+ Misses      16653    16638      -15     
  Partials     2196     2196

Flag	Coverage Δ
unittests-aries-vcx	`30.03% <0.00%> (+0.02%)`	⬆️

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

Files	Coverage Δ
...ibvdrtools/indy-api-types/src/domain/wallet/mod.rs	`7.69% <ø> (ø)`
libvdrtools/indy-api-types/src/errors.rs	`7.98% <ø> (ø)`
libvdrtools/indy-api-types/src/lib.rs	`80.00% <ø> (+30.00%)`	⬆️
...y-utils/src/crypto/chacha20poly1305_ietf/sodium.rs	`62.22% <ø> (ø)`
...rtools/indy-utils/src/crypto/ed25519_box/sodium.rs	`75.00% <ø> (ø)`
...tools/indy-utils/src/crypto/ed25519_sign/sodium.rs	`100.00% <ø> (ø)`
libvdrtools/indy-utils/src/crypto/hash/openssl.rs	`0.00% <ø> (ø)`
...s/indy-utils/src/crypto/pwhash_argon2i13/sodium.rs	`11.53% <ø> (ø)`
...vdrtools/indy-utils/src/crypto/sealedbox/sodium.rs	`66.66% <ø> (ø)`
...bvdrtools/indy-utils/src/crypto/xsalsa20/sodium.rs	`0.00% <ø> (ø)`
... and 57 more

... and 9 files with indirect coverage changes

Patrik-Stas

How about we put all crates under root /crates directory?
"Do we still need agency_client? "

Unfortunately, along with mediated connection, yes. It's needed by NodeJS tests. In aries-vcx, mediated connection module however is not used by other aries-vcx modules, so it could be easily feature flagged, and opt-into from libvcx_core only.
"Remove did_resolver_registry?"

Not sure, probably question for @mirgee
I am wondering if ci can be squashed behind .github - not sure if github allows that
You are missing diddoc_legacy in the described repo structure
I imagine messages_macros wouldn't really be used by other crates other than messages. How about merging messages + messages_macros? Just throwing the idea out there.
libvcx_core is curretly as much/as little legacy as node and vcx-napi-rs components which are built on it. Perhaps they should reside together. I wouldn't mind to but these nodejs wrapper under legacy directory. While we care about them for now, we don't plan to in a long run, and we wouldn't want to encourage usage of it. Also the current agents/node/* would fall into the same category.
- then having nodejs wrappers out of our way from what's in your proposal currently named wrappers, we could "rebrand" that a bit and rather call that "agents" - it could contain:
  - uniffi sample application
  - mediator agent (once's it's chipped into this repo)
  - aries-agent-rust (although not executable, it's a basis for AATH backchannel agent)

bobozaur · 2023-09-26T13:15:13Z

How about we put all crates under root /crates directory?

Idk about this. I personally don't like it. It would make sense if there's a lot of other stuff not related to the Rust code, but there isn't that much really.

2. > "Do we still need agency_client? "
   
   
   Unfortunately, along with mediated connection, yes. It's needed by NodeJS tests. In `aries-vcx`, mediated connection module however is not used by other `aries-vcx` modules, so it could be easily feature flagged, and opt-into from `libvcx_core` only.

Fair enough.

3. > "Remove did_resolver_registry?"
   
   
   Not sure, probably question for @mirgee

To me it just seems like YAGNI.

4. I am wondering if `ci` can be squashed behind `.github` - not sure if github allows that

It might, and it's actually a good idea.

5. You are missing [diddoc_legacy](https://github.com/hyperledger/aries-vcx/tree/main/diddoc_legacy) in the described repo structure

Yaeh, it was meant to go under the legacy dir. Will fix.

6. I imagine `messages_macros` wouldn't really be used by other crates other than `messages`. How about merging `messages` + `messages_macros`? Just throwing the idea out there.

No can do. Procedural macros need to reside in their own crate.

7. `libvcx_core` is curretly as much/as little legacy as `node` and `vcx-napi-rs` components which are built on it. Perhaps they should reside together. I wouldn't mind to but these nodejs wrapper under legacy directory. While we care about them for now, we don't plan to in a long run, and we wouldn't want to encourage usage of it. Also the current `agents/node/*` would fall into the same category.
   
   * then having nodejs wrappers out of our way from what's in your proposal currently named `wrappers`, we could "rebrand" that a bit and rather call that "agents" - it could contain:
     
     * uniffi sample application
     * mediator agent (once's it's chipped into this repo)
     * aries-agent-rust (although not executable, it's a basis for AATH backchannel agent)

Okay, I'll update the directory structure.

mirgee · 2023-09-26T13:15:52Z

I would also prefer putting all (or most) crates into one directory and agree with keeping libvcx_core, vcx-napi-rs and the node wrapper together (not so sure about the node-based agent). did_resolver_registry is used in #928 .

bobozaur · 2023-09-26T13:32:49Z

I would also prefer putting all (or most) crates into one directory and agree with keeping libvcx_core, vcx-napi-rs and the node wrapper together (not so sure about the node-based agent). did_resolver_registry is used in #928 .

Alright so since you both want some upper dir I added it. What's up with the did_resolver_registry though? I know this might not be the proper place to ask but shouldn't consumers be able to pass whatever resolver they want (implementor of DidResolvable)?

The did_resolver_registry is just an opinionated implementation of a resolver that delegates the resolution to underlying, method dedicated resolvers.

bobozaur · 2023-09-26T13:55:05Z

@mirgee If you're adamant on keeping the did_resolver_registry crate please let me know where you place it because I cannot find an appropriate place. I'd say under tools, maybe?

mirgee · 2023-09-26T14:08:25Z

@bobozaur Why not put it under crates/did or just crates.

bobozaur · 2023-09-26T14:09:12Z

@bobozaur Why not put it under crates/did or just crates.

Okay, fair.

Signed-off-by: Bogdan Mircea <mirceapetrebogdan@gmail.com>

bobozaur · 2023-09-28T11:30:52Z

As discussed in the community call, the crates root dir was removed.

Patrik-Stas · 2023-11-01T16:18:45Z

I would like to propose some further changes to the org

.github
docs
aries
- shared_vcx
- legacy
  - did_doc_legacy
  - libvdrtools
  - agency_client
- agents
  - rust
  - node
- aries_vcx
- aries_messages
  - messages
  - messages_macros
- wrappers
  - node
    - libvcx_core
    - node
    - vcx-napi-rs
  - uniffi-aries-vcx
aries_vcx_core (rename)
- vcx_wallet
- vcx_ledger
- vcx_anoncreds
did_document
- did_doc
- did_doc_core
- did_doc_sov
did
- public_key
- did_parser
- did_methods
  - did_resolver_core
  - did_key
  - did_peer
  - did_sov
  - did_web
  - did_registry
tools
- simple_message_relay
- wallet_migrator
There's a few "final state" assumptions made here, for example I am assuming that we split aries_vcx_core to 3 crates vcx_wallet, vcx_ledger, vcx_anoncreds
I've slightly renamed some of the did methods crates, eg did_resolver_sov to just did_sov
Not sure If I haven't misplaced public_key or did_key

But what this mainly does is clearly separates aries components and other components which are reusable and aries-agnostic.

Patrik-Stas · 2023-11-02T14:18:41Z

Moving this discussion to github issue
#998

bobozaur requested review from Patrik-Stas, mirgee and gmulhearn September 26, 2023 10:31

bobozaur self-assigned this Sep 26, 2023

bobozaur force-pushed the update/crates_organization branch from f09ef45 to 4f19de3 Compare September 26, 2023 10:36

Patrik-Stas requested changes Sep 26, 2023

View reviewed changes

bobozaur requested a review from Patrik-Stas September 26, 2023 13:33

bobozaur force-pushed the update/crates_organization branch from 0ebd833 to 583986f Compare September 26, 2023 13:42

bobozaur force-pushed the update/crates_organization branch from 583986f to 751a146 Compare September 27, 2023 09:17

bobozaur added 4 commits September 27, 2023 14:59

Removed obsolete rustfmt.toml files

c202b65

Signed-off-by: Bogdan Mircea <mirceapetrebogdan@gmail.com>

Ran cargo fmt

fb63a58

Signed-off-by: Bogdan Mircea <mirceapetrebogdan@gmail.com>

Moved ci under .github dir

1da02f4

Signed-off-by: Bogdan Mircea <mirceapetrebogdan@gmail.com>

Remove left-overs from other aries_vcx wrappers

b068a6d

Signed-off-by: Bogdan Mircea <mirceapetrebogdan@gmail.com>

bobozaur force-pushed the update/crates_organization branch from 751a146 to b068a6d Compare September 27, 2023 11:59

Patrik-Stas mentioned this pull request Oct 16, 2023

Add mediator and simple client implementation #977

Merged

Patrik-Stas mentioned this pull request Nov 2, 2023

Reorganize repo directories #1045

Closed

Patrik-Stas closed this Nov 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Workspace reorganization #998

Workspace reorganization #998

bobozaur commented Sep 26, 2023 •

edited

Loading

codecov-commenter commented Sep 26, 2023 •

edited

Loading

Patrik-Stas left a comment

bobozaur commented Sep 26, 2023

mirgee commented Sep 26, 2023 •

edited

Loading

bobozaur commented Sep 26, 2023

bobozaur commented Sep 26, 2023

mirgee commented Sep 26, 2023

bobozaur commented Sep 26, 2023

bobozaur commented Sep 28, 2023

Patrik-Stas commented Nov 1, 2023 •

edited

Loading

Patrik-Stas commented Nov 2, 2023

Workspace reorganization #998

Workspace reorganization #998

Conversation

bobozaur commented Sep 26, 2023 • edited Loading

codecov-commenter commented Sep 26, 2023 • edited Loading

Codecov Report

Patrik-Stas left a comment

Choose a reason for hiding this comment

bobozaur commented Sep 26, 2023

mirgee commented Sep 26, 2023 • edited Loading

bobozaur commented Sep 26, 2023

bobozaur commented Sep 26, 2023

mirgee commented Sep 26, 2023

bobozaur commented Sep 26, 2023

bobozaur commented Sep 28, 2023

Patrik-Stas commented Nov 1, 2023 • edited Loading

Patrik-Stas commented Nov 2, 2023

bobozaur commented Sep 26, 2023 •

edited

Loading

codecov-commenter commented Sep 26, 2023 •

edited

Loading

mirgee commented Sep 26, 2023 •

edited

Loading

Patrik-Stas commented Nov 1, 2023 •

edited

Loading