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 instructions for creating a PDF of the documentation #1178

Merged
merged 2 commits into from
Oct 14, 2024

Conversation

skrawcz
Copy link
Collaborator

@skrawcz skrawcz commented Oct 13, 2024

I believe the docs requirements are correct to get this to work.

i.e. add:

pip install sphinx-simplepdf

then

sphinx-build -b simplepdf -W -E -T -a docs /tmp/mydocs

from the root of the repository. It should take a few minutes then to build a PDF that will turn up under /tmp/mydocs.

Changes

  • docs/conf.py

How I tested this

  • locally

Notes

Checklist

  • PR has an informative and human-readable title (this will be pulled into the release notes)
  • Changes are limited to a single goal (no scope creep)
  • Code passed the pre-commit check & code is left cleaner/nicer than when first encountered.
  • Any change in functionality is tested
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged / future TODOs are captured in comments
  • Project documentation has been updated if adding/changing functionality.

I believe the docs requirements are correct to get this to work.

i.e. add:

`pip install sphinx-simplepdf`

then

`sphinx-build -b simplepdf  -W -E -T  -a docs /tmp/mydocs`

from the root of the repository. It should take a few minutes
then to build a PDF that will turn up under `/tmp/mydocs`.
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to 0f52fe6 in 6 seconds

More details
  • Looked at 60 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. README-DOCS.md:28
  • Draft comment:
    Consider adding sphinx-simplepdf to the installation command in the documentation to ensure users have all necessary dependencies.
pip install .[docs] sphinx-simplepdf
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The addition of the sphinx-simplepdf package in the pyproject.toml file is correct and aligns with the PR's intent to add PDF generation capabilities. However, the PR description mentions adding pip install sphinx-simplepdf to the documentation, but this is not reflected in the README-DOCS.md. This could lead to confusion for users trying to follow the instructions.
2. README-DOCS.md:27
  • Draft comment:
    Consider adding these instructions to the docs/ directory to ensure they are included in the Sphinx documentation.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR adds instructions for creating a PDF of the documentation, which is a useful addition. However, the new functionality should be documented in the docs/ directory to ensure users can find it easily.

Workflow ID: wflow_LaqKKKDHS3Obydhz


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on b957f1d in 9 seconds

More details
  • Looked at 22 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. .readthedocs.yaml:21
  • Draft comment:
    The formats key is not valid in the Read the Docs configuration file. Consider removing it or checking the documentation for valid keys.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The comment directly addresses a change made in the diff, specifically the addition of the 'formats' key. The comment suggests checking the documentation, which implies that the 'formats' key might not be valid. This is a potential issue that could affect the build process if the key is indeed invalid.
    I might be missing the latest updates to the Read the Docs configuration specifications. The 'formats' key could be a new addition that the comment is not aware of.
    I should verify the validity of the 'formats' key against the latest Read the Docs documentation to ensure the comment is accurate.
    Keep the comment for now, as it addresses a change made in the diff and suggests a potential issue with the configuration file.
2. .readthedocs.yaml:21
  • Draft comment:
    Consider adding a section in the docs/ to document the process of generating a PDF using the Read the Docs configuration, as this change introduces the capability to build PDF documentation.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR is about adding instructions for creating a PDF of the documentation, and the change in the .readthedocs.yaml file is relevant to this. The addition of the 'formats' section with 'pdf' is appropriate for the goal of the PR.

Workflow ID: wflow_B4cSP26SxRurQ7bb


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Let's see if this works.
@skrawcz skrawcz force-pushed the add_pdf_instructions branch from b957f1d to 3b0ef3a Compare October 14, 2024 00:43
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on 3b0ef3a in 3 seconds

More details
  • Looked at 14 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. .readthedocs.yaml:20
  • Draft comment:
    Ensure that the 'sphinx-simplepdf' package is included in the 'docs' extra_requirements in your setup configuration to avoid build issues.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The addition of the 'formats' section in the .readthedocs.yaml file is correct and aligns with the PR description. It specifies that a PDF format should be generated, which is the intent of the PR.
2. .readthedocs.yaml:20
  • Draft comment:
    Consider adding a note in the docs/ directory documentation about the new PDF format support in the Read the Docs configuration.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The addition of the PDF format in the Read the Docs configuration is appropriate for generating PDF documentation.

Workflow ID: wflow_kv1KcdRurlgWWt5D


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@skrawcz skrawcz merged commit 41c1a15 into main Oct 14, 2024
22 of 24 checks passed
@skrawcz skrawcz deleted the add_pdf_instructions branch October 14, 2024 00:49
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.

1 participant