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

Add architecture.md and module-level documentation #1556

Merged
merged 16 commits into from
Nov 19, 2021
Merged

Add architecture.md and module-level documentation #1556

merged 16 commits into from
Nov 19, 2021

Conversation

seanchen1991
Copy link
Contributor

@seanchen1991 seanchen1991 commented Nov 9, 2021

Second version of #1341

Description

Adds an architecture.md doc that gives a high-level overview of the structure of ibc-rs. This doc was written in such a way as to hopefully not require much, if any, updating going forward; we've tried to document invariants and patterns that will hold in perpetuity.

Some module-level documentation has also been added.


For contributor use:

  • Added a changelog entry, using unclog.
  • If applicable: Unit tests written, added test to CI.
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Updated relevant documentation (docs/) and code comments.
  • Re-reviewed Files changed in the Github PR explorer.

@seanchen1991 seanchen1991 added O: code-hygiene Objective: cause to improve code hygiene I: documentation Internal: improvements or additions to documentation labels Nov 9, 2021
Copy link
Member

@adizere adizere left a comment

Choose a reason for hiding this comment

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

We're pretty much there! I left a few minor suggestions, please have a look.

.changelog/unreleased/improvements/1556-arch-doc.md Outdated Show resolved Hide resolved
docs/architecture/architecture.md Show resolved Hide resolved
docs/architecture/architecture.md Show resolved Hide resolved
docs/architecture/architecture.md Outdated Show resolved Hide resolved
docs/architecture/architecture.md Show resolved Hide resolved
@adizere
Copy link
Member

adizere commented Nov 17, 2021

I think this PR aims to provide guidance to first-time contributors and colleagues onboarding (among others). With that in mind, I'm wondering if @mpoke or @mzabaluev have any thoughts on the architecture.md doc introduced in this PR. Thank you in advance Marius or Mikhail for any feedback!

Copy link
Member

@adizere adizere left a comment

Choose a reason for hiding this comment

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

Approving pre-emptively here. Great work Sean! Might be a good idea to wait to merge until Marius & Mikhail have a chance to peek! I think their feedback might be very useful since they are the target audience of docs/architecture/architecture.md.

docs/architecture/architecture.md Outdated Show resolved Hide resolved
Copy link
Contributor

@mzabaluev mzabaluev left a comment

Choose a reason for hiding this comment

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

Very informative, thank you!

docs/architecture/architecture.md Show resolved Hide resolved
@mzabaluev
Copy link
Contributor

This comment was somehow attached to a resolved thread, repeating it here:

The image has transparent background, some elements are not visible with a Dark theme on GitHub or any other viewer that might use a dark background.

Copy link

@mpoke mpoke left a comment

Choose a reason for hiding this comment

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

I think the document is very useful. Thank you.

@mzabaluev
Copy link
Contributor

ics23 only gets a mention in the diagram, but not otherwise explained.

Overall, the document looks good.

@seanchen1991 seanchen1991 merged commit 9160dce into informalsystems:master Nov 19, 2021
@seanchen1991 seanchen1991 deleted the arch-doc branch November 19, 2021 15:56
hu55a1n1 pushed a commit to hu55a1n1/hermes that referenced this pull request Sep 13, 2022
)

* Add arch doc with mention of flex-error

* Add module-level documentation

* Add some more module-level documentation

* Add some more module-level documentation

* Add more module-level documentation

* Add Fungible Token Transfer section to arch.md

* Fix broken markdown link

* Add changelog entry

* Run cargo fmt

* Incorporate PR feedback

* Replace `ibc-rs-layout` image

* Fix broken markdown link

* Fix broken markdown links

* Replace layout image

* Change header ordering
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I: documentation Internal: improvements or additions to documentation O: code-hygiene Objective: cause to improve code hygiene
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants