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

docs: move shares README to godoc #842

Merged
merged 1 commit into from
Oct 12, 2022
Merged

Conversation

rootulp
Copy link
Collaborator

@rootulp rootulp commented Oct 6, 2022

Motivated by celestia-node's package-specific-documentation, I opted to move the shares package README.md to godoc. Some changes were necessary because godoc doesn't support all Markdown features. Also, I updated "message start" terminology to use "sequence start" because it applies to non-message shares.

Screenshot

Screen Shot 2022-10-05 at 10 43 10 PM

@rootulp rootulp added the documentation Improvements or additions to documentation label Oct 6, 2022
@rootulp rootulp requested a review from rach-id October 6, 2022 02:47
@rootulp rootulp self-assigned this Oct 6, 2022
@rootulp rootulp enabled auto-merge (squash) October 6, 2022 02:47
@codecov-commenter
Copy link

Codecov Report

Merging #842 (3750933) into main (d8a9272) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #842   +/-   ##
=======================================
  Coverage   25.63%   25.63%           
=======================================
  Files          75       75           
  Lines        9279     9279           
=======================================
  Hits         2379     2379           
  Misses       6677     6677           
  Partials      223      223           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Member

@rach-id rach-id left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks a lot for taking care of this. But, I would defer to Evan, as I never used godoc before.

@rootulp
Copy link
Collaborator Author

rootulp commented Oct 6, 2022

Sounds good, we can wait for Evan's review.

To clarify godoc is the standard tool to that generate go documentation. It is used for https://pkg.go.dev/std and https://pkg.go.dev/github.com/celestiaorg/celestia-app and for most other Go projects that have docs

@rach-id
Copy link
Member

rach-id commented Oct 6, 2022

Interesting, thanks, didn't know we were publishing docs

Copy link
Member

@evan-forbes evan-forbes left a comment

Choose a reason for hiding this comment

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

I'm unsure of this suggestion, so please feel free to ignore or modify:

From my own personal workflow, I appreciate seeing markdown files in github/text editors. Markdown files are also easier to edit than go comments and all of our other docs just use markdown. Using godocs is also super legit. Do we know of a way to do both, have a markdown file that gets pushed to go docs? if not nbd, we can just use godocs or maybe include a readme that links to the godocs?

@rootulp rootulp merged commit dd02bb8 into celestiaorg:main Oct 12, 2022
@rootulp
Copy link
Collaborator Author

rootulp commented Oct 12, 2022

I'd rather read and edit Markdown than Godoc comments so I agree with your concern. It seems like a README.md is preferable for developers contributing to this repo but Godoc comments are preferable for developers using the share package as a dependency because the Godoc comment will be more discoverable than a README.md in the share package.

I'm not opposed to a README.md in the share package that links to the Godoc comments but I'm more inclined to copy celestia-node and include a section like https://github.com/celestiaorg/celestia-node#package-specific-documentation in our root README.md because that's more discoverable

Update: disregard, it looks like a README surfaces in Godocs. Example: https://pkg.go.dev/github.com/celestiaorg/celestia-app@v0.7.0-rc5/pkg/shares#section-readme

rootulp added a commit that referenced this pull request Oct 12, 2022
rootulp added a commit that referenced this pull request Oct 13, 2022
rach-id pushed a commit to rach-id/celestia-app that referenced this pull request Nov 16, 2022
Motivated by celestia-node's
[package-specific-documentation](https://github.com/celestiaorg/celestia-node#package-specific-documentation),
I opted to move the shares package `README.md` to godoc. Some changes
were necessary because godoc doesn't support all Markdown features.
Also, I updated "message start" terminology to use "sequence start"
because it applies to non-message shares.

## Screenshot

<img width="560" alt="Screen Shot 2022-10-05 at 10 43 10 PM"
src="https://user-images.githubusercontent.com/3699047/194202854-682e044a-3c5f-4f09-bbe3-6c8b74625f05.png">
rach-id pushed a commit to rach-id/celestia-app that referenced this pull request Nov 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants