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

Adds docs for readthedocs #8

Merged
merged 1 commit into from
Jun 11, 2021

Conversation

qduk
Copy link
Collaborator

@qduk qduk commented May 14, 2021

This now dynamically pulls from the README for the respective section. Was also able to fix the decorated functions and use invoke instead of make. Now, in the docs folder you can you just run invoke html and it'll build your html files.

Edit:
To create files locally.

  1. Install requirements under docs folder `pip install -r docs/requirements.txt
  2. Run invoke html in project root.
  3. File will be generated in docs/build

.. toctree::
:maxdepth: 2

asn/index
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is config?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The config for asn? It lives in its own folder under source/netutils/asn.

Copy link
Contributor

Choose a reason for hiding this comment

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

like should there be an config/index?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Totally missed those. I added a general configs tab. We could break it up how the code is with each file having its own tab under config. Let me know what you think and I can change it if need be.

docs/tasks.py Outdated Show resolved Hide resolved
docs/tasks.py Outdated Show resolved Hide resolved
netutils/mac.py Outdated Show resolved Hide resolved
@itdependsnetworks
Copy link
Contributor

Good stuff!

We should add a test or two.

  • Ensure it runs and compiles.
  • I think the end-line/start-line is great, but suspect that will move over time, can compare it against current standard.
  • See if there is a way to verify that all files are considered.

@qduk
Copy link
Collaborator Author

qduk commented May 18, 2021

@itdependsnetworks I added a few tests. I'm not sure if they should live in the unit tests folder or not but they check the following,

  • Sphinx builds without any errors
  • Checks that there is a folder in docs/source/netutils for a given python file
  • Each folder contains an index.

There is a PR that would've allowed me to use the start-before and end-after directives which would've been easier to break the README into sections but it was never merged into the project. So the line directives seemed like the best next bet.

@qduk qduk requested a review from itdependsnetworks May 18, 2021 23:16
docs/tasks.py Outdated Show resolved Hide resolved
@qduk qduk requested a review from itdependsnetworks June 5, 2021 06:06
sourcedir (str, optional): Source directory for sphinx to use. Defaults to "source".
builddir (str, optional): Output directory for sphinx to use. Defaults to "build".
"""
print("Building html documentation...")
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it makes sense to call clean_docs here, just ensure it is done idempotently.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated

"build_dir": "docs/build/",
}
]

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a start/end line test here? Something to ensure that the current start and end line remain the same throughout updates. This will ensure that if the readme is modified, that the tests will fail. If the start or end line change, they simply need to modify the tests.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added tests to check for that. Let me know what you think.

Comment on lines 38 to 45
autodoc_default_options = {
# "exclude-members": "__weakref__",
# "ignore-module-all": ,
# "imported-members": ,
# "inherited-members": ,
"members": True,
# "member-order": "bysource",
# "private-members": ,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does it make sense to keep these?, being that they're just commented out?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Took them out. Good call. I believe Sphinx makes it that way when you start with it's template. Thanks Jeff.

assert "index.rst" in os.listdir(folder)


def test_overview_section_start_end_lines(load_readme_into_lines): # pylint: disable=redefined-outer-name
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we just create a data structure, and a single test. Something like

[
  {
    "overview": {
      "start-line": 10,
      "start-val": "# netutils\n",
      "end-line": 32,
      "end-value": "* VLANs - Provide the ability to convert configuration into lists or lists into configuration.\n"
    }
  },
  {
    "attribution": {
      "start-line": "NN",
      "start-val": "TEXT",
      "end-line": "NN",
      "end-value": "TEXT"
    }
  }
]

Even better would be if you can dynamically find the line numbers, that are shown here, as an example:

   :start-line: 131   :end-line: 178

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated. I created an issue for parsing the start and end line so I can get that handled down the road.

@qduk qduk requested a review from itdependsnetworks June 10, 2021 18:33
@itdependsnetworks itdependsnetworks merged commit 6e2e5c5 into networktocode:develop Jun 11, 2021
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.

3 participants