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

Enhance aries_vcx crate readme #1043

Merged
merged 3 commits into from
Nov 6, 2023
Merged

Enhance aries_vcx crate readme #1043

merged 3 commits into from
Nov 6, 2023

Conversation

Patrik-Stas
Copy link
Contributor

@Patrik-Stas Patrik-Stas commented Oct 31, 2023

  • Rewrote aries_vcx crate readme
  • Rename aries-vcx package to aries_vcx, looking at most of are other crates, it seems that underscore is "the way" :-) I like it though

Also want to point out the distinction between aries-vcx and aries_vcx:

  • aries-vcx being name of the HL project currently containing variety of useful components, not particularly tied to aries itself - such as crates did, did_doc, did_peer and number of others
  • aries_vcx is the main, but also just one of many crates in the repo

@Patrik-Stas Patrik-Stas marked this pull request as ready for review October 31, 2023 17:26
@Patrik-Stas Patrik-Stas changed the title Enhance aries_vcx crate readmes Enhance aries_vcx crate readme Oct 31, 2023
@codecov-commenter
Copy link

codecov-commenter commented Oct 31, 2023

Codecov Report

Merging #1043 (cf09a1c) into main (63590d1) will not change coverage.
Report is 1 commits behind head on main.
The diff coverage is n/a.

❗ Current head cf09a1c differs from pull request most recent head 3ac5751. Consider uploading reports for the commit 3ac5751 to get more accurate results

@@          Coverage Diff          @@
##            main   #1043   +/-   ##
=====================================
  Coverage   0.05%   0.05%           
=====================================
  Files        384     384           
  Lines      21249   21249           
  Branches    3835    3835           
=====================================
  Hits          12      12           
  Misses     21236   21236           
  Partials       1       1           
Flag Coverage Δ
unittests-aries-vcx 0.05% <ø> (ø)

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

Signed-off-by: Patrik Stas <[email protected]>
@nain-F49FF806
Copy link
Member

nain-F49FF806 commented Nov 1, 2023

I think this is a nice change. It gives a more clear and relevant at glance view to someone approaching aries_vcx afresh. Combined with the readme on the project.

The change to underscores is welcome consistency. And should offer slightly lower cognitive effort keeping track of delimiters.

On the other front, I think we could do more to differentiate aries-vcx (project) vs aries_vcx (crate),
which may be confusing for someone not yet familiar with the project's codebase.

Is it feasible and appropriate to rename the project as "Aries VCX" / "AriesVCX" ?
Capitalisation would help better distinguish between the overarching project (Title Case) and individual crates of code (snake_case).

ACA-Py for example has hyphenated repo but title case project name. Similarly with Firefly.
On casual glance, one other hyperledger repo that retains kebab-case seems to be a single crate library.
anoncreds-rs,

@Patrik-Stas
Copy link
Contributor Author

Patrik-Stas commented Nov 1, 2023

Is it feasible and appropriate to rename the project as "Aries VCX" / "AriesVCX" ?

A long time ago we came to agreement we will call the repo "aries-vcx", there was a time of many inconsistencies...at the time of that decision, the repo was pretty much ONLY aries_vcx the crate. We had a thoughts lingering recently about bigger change in term of repo name, but I think what you propose is reasonable change, making the repo more human readable, less technical - eg "Aries VCX" / "AriesVCX". Just not sure which one of the two.

Copy link
Contributor

@gmulhearn gmulhearn left a comment

Choose a reason for hiding this comment

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

Change sounds good to me!

@Patrik-Stas Patrik-Stas merged commit 4e2a5f4 into main Nov 6, 2023
28 checks passed
@Patrik-Stas Patrik-Stas deleted the docs/readme-update branch November 6, 2023 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants