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 documentation #330

Closed
wants to merge 8 commits into from
Closed

Add documentation #330

wants to merge 8 commits into from

Conversation

dmuhs
Copy link
Contributor

@dmuhs dmuhs commented Oct 23, 2019

This PR will

  • add Sphinx as a documentation system
  • update the docs with autodoc instructions for all components of the code base
  • add basic installation instructions to the docs
  • fix a minor indentation issue in the Swarm.listen docstring

With this PR merged, documentation can be built locally by entering the docs folder and using the Makefile:

cd docs && make html

The output can be found in the build directory, in the case of html docs, it can be opened at docs/build/html/index.html. A variety of other documentation formats are available and can be checked out by using the Makefile's help command:

$ make help
Sphinx v2.2.0
Please use `make target' where target is one of
  html        to make standalone HTML files
  dirhtml     to make HTML files named index.html in directories
  singlehtml  to make a single large HTML file
  pickle      to make pickle files
  json        to make JSON files
  htmlhelp    to make HTML files and an HTML help project
  qthelp      to make HTML files and a qthelp project
  devhelp     to make HTML files and a Devhelp project
  epub        to make an epub
  latex       to make LaTeX files, you can set PAPER=a4 or PAPER=letter
  latexpdf    to make LaTeX and PDF files (default pdflatex)
  latexpdfja  to make LaTeX files and run them through platex/dvipdfmx
  text        to make text files
  man         to make manual pages
  texinfo     to make Texinfo files
  info        to make Texinfo files and run them through makeinfo
  gettext     to make PO message catalogs
  changes     to make an overview of all changed/added/deprecated items
  xml         to make Docutils-native XML files
  pseudoxml   to make pseudoxml-XML files for display purposes
  linkcheck   to check all external links for integrity
  doctest     to run all doctests embedded in the documentation (if enabled)
  coverage    to run coverage check of the documentation (if enabled)

New documentation can be added by simply manipulating the code components' docstrings, or explicitly by editing the rst files in the docs folder. These files mirror 1:1 the module structure of the library at the moment and should be adapted in case larger refactorings take place. This makes it easy for developers already familiar with the project structure to also contribute to the documentation if further explanation is needed.

The goal of this PR is to lay the foundation for further exhaustive documentation of each code component (ref. #318, #90, #35), as well as improving docstring quality and guiding future refactors leading py-libp2p to a state where we can remove the WIP label from it. 🙂

@dmuhs
Copy link
Contributor Author

dmuhs commented Oct 23, 2019

Added fix for mypy error, taken from d52b093

Copy link
Contributor

@mhchia mhchia left a comment

Choose a reason for hiding this comment

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

Awesome! I tried and it works well. However, I'm not familiar with the docs setup. It will be great to have @carver 's review, regarding #294.

@pipermerriam
Copy link
Contributor

cc @cburgdorf who is our resident documentation expert.

@carver
Copy link
Contributor

carver commented Oct 25, 2019

For reference, here's an excerpt of the global makefile we use to build docs in Trinity: https://github.com/ethereum/trinity/blob/24213d8f22a03c370313d3a990eea6a2fe1e76e4/Makefile#L43-L57

I'm happy to take a look at this PR tomorrow.

Copy link
Contributor

@carver carver left a comment

Choose a reason for hiding this comment

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

Now that I look deeper, I'm seeing a bunch of things that I would like to see added, like towncrier support for release notes. If you'd like, I can pick it up from here and add more of these things.

@@ -146,7 +146,7 @@ def is_initiator(self) -> bool:
:return: number of bytes written
"""
if self.event_local_closed.is_set():
raise MplexStreamClosed(f"cannot write to closed stream: data={data}")
raise MplexStreamClosed(f"cannot write to closed stream: data={data!r}")
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd recommend this one to be moved to another PR, it should be a very quick 👍

Copy link
Contributor Author

@dmuhs dmuhs Oct 26, 2019

Choose a reason for hiding this comment

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

It seems like this is already resolved in master with d52b093. I have merged it into my PR branch and the changes are resolved correctly.

libp2p/utils.py Outdated
@@ -77,7 +77,7 @@ def encode_delim(msg: bytes) -> bytes:
raise ParseError(f"`len(msg_bytes)` should not be 0")
if msg_bytes[-1:] != b"\n":
raise ParseError(
f'`msg_bytes` is not delimited by b"\\n": `msg_bytes`={msg_bytes}'
f'`msg_bytes` is not delimited by b"\\n": `msg_bytes`={msg_bytes!r}'
Copy link
Contributor

Choose a reason for hiding this comment

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

This one can also be added to that separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also fixed in d52b093 on master already

setup.py Outdated
@@ -16,7 +16,7 @@
"isort==4.3.21",
"flake8>=3.7.7,<4.0.0",
],
"dev": ["tox>=3.13.2,<4.0.0"],
"dev": ["tox>=3.13.2,<4.0.0", "Sphinx>=2.2.0,<3.0.0", "sphinx-rtd-theme"],
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in f7403ef and added doc dependencies to the dev requirements in 817e9c8

@dmuhs dmuhs requested review from carver October 26, 2019 11:26
@@ -0,0 +1,17 @@
Welcome to py-libp2p's documentation!
Copy link
Contributor

Choose a reason for hiding this comment

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

We have a few general documentation guidelines, and we're trying to make sure that all our repos are as close to them as possible.

That's why I'm taking this opportunity to point this out so that we can get these policies involved early on.

It's far from perfect but the project closest to the ideals of this guide line is probably Trinity so I recommend to take a look at its docs.

In particular, it would be nice if we could roughly follow this navigation structure.

image

General

Introduction: The landing page, giving a quick overview about the project, additional pointers.
Quickstart: Pretty much what you already have with all the installation instructions
Release Notes: Points to the towncrier generated release notes

Fundamentals

API: Source generated API docs as you already have them included in the PR
Cookbook: Collection of small recipes, examples. Doesn't need to exist in the beginning
Guides: Tutorial style guides. It's ok to just have a link to the Quickstart here to begin with

Community

Contributing: Instructions on how to contribute to the project
Code of Conduct: The code of conduct that we use across repos

Stable release
--------------

To install :code:`py-libp2p`:, run this command in your terminal:
Copy link
Contributor

Choose a reason for hiding this comment

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

There's no right or wrong when it comes to the narrative perspective but we chose to use the first-person "we" form which means there is rarely ever a place for "you" in our docs.

E.g. see Trinity's quickstart written in first person "we" form

@pacrob
Copy link
Member

pacrob commented Apr 20, 2024

Docs reorged and more in #454

@pacrob pacrob closed this Apr 20, 2024
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.

6 participants