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

Feat/module docs #83 #267

Merged
merged 37 commits into from
May 29, 2020
Merged

Feat/module docs #83 #267

merged 37 commits into from
May 29, 2020

Conversation

shannonwells
Copy link
Contributor

@shannonwells shannonwells commented May 27, 2020

Documentation pass for modules in this repo.

  • Add README.md to root of each package, filestore, pieceio, piecestore, retrievalmarket, and storagemarket
  • Link to READMEs on main README
  • Write interface implementation documentation for storagemarket and retrievalmarket
  • Write a little bit about filestore, pieceio, and piecestore.

Request

I would appreciate feedback or even better, more detailed content on the storagemarket module since I don't know it as well as retrieval.

@codecov-commenter
Copy link

codecov-commenter commented May 27, 2020

Codecov Report

Merging #267 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #267   +/-   ##
=======================================
  Coverage   64.86%   64.86%           
=======================================
  Files          40       40           
  Lines        2342     2342           
=======================================
  Hits         1519     1519           
  Misses        699      699           
  Partials      124      124           

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 2132c05...2132c05. Read the comment docs.

Copy link
Collaborator

@hannahhoward hannahhoward left a comment

Choose a reason for hiding this comment

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

Submitting these comments early even though it's not ready for review:

Generally the storage & retrieval modules are headed in the right direction, and I just have some suggestions for additional information.

I'm also going to tag @mishmosh on this as someone with experience in documentation.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
piecestore/README.md Outdated Show resolved Hide resolved
piecestore/README.md Outdated Show resolved Hide resolved
piecestore/README.md Outdated Show resolved Hide resolved
retrievalmarket/README.md Show resolved Hide resolved
storagemarket/README.md Outdated Show resolved Hide resolved
@hannahhoward hannahhoward requested a review from mishmosh May 27, 2020 20:41
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
piecestore/piecestore.go Show resolved Hide resolved
retrievalmarket/README.md Outdated Show resolved Hide resolved
@shannonwells shannonwells marked this pull request as ready for review May 28, 2020 18:34
Copy link
Collaborator

@mishmosh mishmosh left a comment

Choose a reason for hiding this comment

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

Latest changes address all of my comments, thanks! Please wait for @hannahhoward’s green check too before merging though.

Copy link
Contributor

@ingar ingar left a comment

Choose a reason for hiding this comment

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

I have a handful of slight changes that might clarify things, and some syntactical nits.

storagemarket/README.md Outdated Show resolved Hide resolved
storagemarket/README.md Outdated Show resolved Hide resolved
storagemarket/README.md Outdated Show resolved Hide resolved
storagemarket/README.md Outdated Show resolved Hide resolved
storagemarket/README.md Outdated Show resolved Hide resolved
storagemarket/README.md Outdated Show resolved Hide resolved
storagemarket/README.md Outdated Show resolved Hide resolved
storagemarket/README.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@hannahhoward hannahhoward left a comment

Choose a reason for hiding this comment

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

Honestly, THIS PR IS AWESOME. LGTM++++

Comment on lines +3 to +4
The `filestore` module is a simple wrapper for os.File. It is used by [pieceio](../pieceio),
[retrievialmarket](../retrievalmarket), and [storagemarket](../storagemarket).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would add the following:

Suggested change
The `filestore` module is a simple wrapper for os.File. It is used by [pieceio](../pieceio),
[retrievialmarket](../retrievalmarket), and [storagemarket](../storagemarket).
The `filestore` module is a simple wrapper for os.File. It's primary purpose to wrap a specific local directory configured by the node. It is used by [pieceio](../pieceio),
[retrievialmarket](../retrievalmarket), and [storagemarket](../storagemarket).

@shannonwells shannonwells merged commit 2cd5ecc into master May 29, 2020
@shannonwells shannonwells deleted the feat/module-docs-#83 branch May 29, 2020 17:32
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.

5 participants