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

Reorganize the ACA-Py Documentation Files #2765

Merged
merged 3 commits into from
Feb 9, 2024
Merged

Reorganize the ACA-Py Documentation Files #2765

merged 3 commits into from
Feb 9, 2024

Conversation

swcurran
Copy link
Contributor

@swcurran swcurran commented Feb 7, 2024

This PR does a radical reorganization of the documents in the ACA-Py repository. The following is a summary of the changes:

  • Most Markdown files in the root have been moved to the docs folder, and organized into folders that match those used on the https://aca-py.org -- demo, gettingStarted, testing, deploying, etc.
  • Expected files -- LICENSE, MAINTAINERS.md, README.md and the like were left in the root folder.
  • The MD files in the demo folder were moved to `docs/demo' and a new README.md file added pointing to those.
  • The previous Getting Started files were moved to the docs/gettingStarted folders.
  • A pass was done to change all the links found (in a manual pass...) to be relative links and to the new location of the files.
  • A pass was done to remove all of the MD lint warnings (other than spelling of ACA-Py/Aries/SSI terms).
  • Some cleanup was done to external references -- changing references from greenlight ledger to BCovrin test, eliminating some references to old demos, adding references to the Traction Tutorial.
  • New README.md files were needed -- such as in the root of the docs folder with pointers to guidance for maintaining both the ReadTheDocs documentation and the https://aca-py.org documentation.
  • I've not done anything with the ELK Stack MD documents -- will think about what to do with those later (if anything).

Not perfect, but I think it is close.

It is a lot of change. Would appreciate people browsing my branch in the GitHub UI to scan for issues, and report them. Important things to note are changes that need to be made before we merge this and more general updates that are overdue and needed.

@swcurran swcurran requested review from ianco, dbluhm and jamshale February 7, 2024 22:38
@swcurran
Copy link
Contributor Author

swcurran commented Feb 7, 2024

Not good that we have an integration test error when moving docs :-).

The error is from this test: features/0586-sign-transaction.feature:204 endorse a schema and cred def transaction, write to the ledger, issue and revoke a credential, with auto endorsing workflow -- @1.2

Not sure if it is a timing issue or something else. Seems to be finding duplicate records. Perhaps someone could run this locally. It is related to the Endorser, so perhaps updates in that area?

README.md Outdated Show resolved Hide resolved
PUBLISHING.md Outdated Show resolved Hide resolved
@jamshale
Copy link
Contributor

jamshale commented Feb 7, 2024

Not sure if it is a timing issue or something else. Seems to be finding duplicate records. Perhaps someone could run this locally. It is related to the Endorser, so perhaps updates in that area?

I'm pretty sure this is an intermittently failing test, as other tests off of main are passing and I think I've seen it before. Re-running. If it fails again I'll look at it locally.

Copy link

sonarqubecloud bot commented Feb 8, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@swcurran
Copy link
Contributor Author

swcurran commented Feb 8, 2024

Turned on the VSCode link checker (who knew that wouldn't be on automatically!) and did a pass through all of the MDs again -- cleaned up links and all MD lint issues. Only thing left are the a few bare URLs (email addressess...) and the unlnown words that would be expected. I think that clean up is now done.

Copy link
Contributor

@dbluhm dbluhm left a comment

Choose a reason for hiding this comment

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

LGTM! Truthfully, I didn't read through every change but I'll rely on @jamshale's more in depth review and the linting to catch issues with links and such.

Copy link
Contributor

@dbluhm dbluhm left a comment

Choose a reason for hiding this comment

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

Meant to hit approve...

@ianco
Copy link
Contributor

ianco commented Feb 9, 2024

Not sure if it is a timing issue or something else. Seems to be finding duplicate records. Perhaps someone could run this locally. It is related to the Endorser, so perhaps updates in that area?

I'm pretty sure this is an intermittently failing test, as other tests off of main are passing and I think I've seen it before. Re-running. If it fails again I'll look at it locally.

I've noticed the occasional timing issue with the ledger. I.e. the test registers a DID and then when it tries to record a schema it get an error that the DID isn't approved to update the ledger. I think it's a node synchronization timing issue.

Copy link
Contributor

@ianco ianco left a comment

Choose a reason for hiding this comment

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

Overall looks good. (Like Daniel I didn't read through every single document.) My preference would be to leave the demo docs in the /demo folder, but I don't have a strong opinion about it.

@swcurran
Copy link
Contributor Author

swcurran commented Feb 9, 2024

I can’t believe y’all didn’t read every change in every file!!! IMHO, we want to encourage people to use https://aca-py.org, so having a redirector page in the /demo folder is the right thing to do. Understand your concerns, but at least they are all in one place — /docs/demo.

Thanks for the reviews!

@swcurran swcurran merged commit b7a9176 into openwallet-foundation:main Feb 9, 2024
8 checks passed
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