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 API reference generated by Sphinx #212

Merged
merged 6 commits into from
Nov 5, 2024

Conversation

Hoolean
Copy link
Contributor

@Hoolean Hoolean commented Sep 22, 2024

Hello - I've had a stab at #206!

This is largely a modification of sphinx-quickstart to automatically generate an API reference for the module, taking a few hints from fonttools/ufoLib2 to inform structure, build workflow, and styling.

It can be tested as follows:

tox -e docs

...and seems to be fairly happy with Cython, aside from a known upstream issue with displaying the types of properties (sphinx-doc/sphinx#7448).

In general, the generated documentation will remain sparse until more docstrings are added, however, it will hopefully still deliver an improvement over reading the sources, especially with the built-in search functionality that Sphinx provides :)

Known caveats:

  • Author and derived copyright are lifted from setup.py; are these the most accurate to use?
  • No CI integration

Note: this is a personal contribution independent of my employer, and so I've submitted from a fork under my personal profile and email to make this distinction ✔️

This is `sphinx-quickstart` as modified to automatically produce
documentation for the module, taking a few hints from fonttools/ufoLib2
to inform structure, build workflow, and styling.
Having an explicit __all__ is necessary to avoid re-exporting objects
that we ourselves have imported (e.g. from the standard library).

This also helps avoid objects that this project did not author from
appearing in the automatically generated Sphinx documentation.

This list was generated by:

```python
import uharfbuzz
print(dir(uharfbuzz))
```

...before being manually pruned.
Copy link
Collaborator

@khaledhosny khaledhosny 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, thanks!. CI integration would be welcome. We probably should also publish it to readthedocs.io.

src/uharfbuzz/__init__.py Show resolved Hide resolved
docs/source/conf.py Outdated Show resolved Hide resolved
@khaledhosny khaledhosny linked an issue Sep 24, 2024 that may be closed by this pull request
@Hoolean
Copy link
Contributor Author

Hoolean commented Sep 24, 2024

Thanks for the speedy review Khaled 💪 I'll see what it would take to run the tox command alongside the project tests, and/or construct a basic .readthedocs.yml

This is following the guidance at:

    https://docs.readthedocs.io/en/stable/guides/reproducible-builds.html

...in preparation for adding Read the Docs build support, where having
an explicit requirements.txt file to point to will also be helpful.
This is based on the Sphinx reference config file here:

    https://docs.readthedocs.io/en/stable/config-file/v2.html#python-install

...with some trimming, customisation for the project, and additional
adjustments recommended for reproducible builds.
@Hoolean
Copy link
Contributor Author

Hoolean commented Sep 24, 2024

I've added a template .readthedocs.yml, and have tried to use options cautiously until we are able to test in a real environment; in particular, I am not sure how the doc's dependency on a compiled wheel will be handled.

Would someone with maintainer access be willing to follow the Read the Docs procedure required to create a sub-domain and have it insert actions into this repository to populate the docs?

Side note: if the pull request option is available for community plans then we may be able to avoid the duplicate runner effort of including a CI job for docs on GitHub too 🚀

@khaledhosny
Copy link
Collaborator

I’m merging this now to see how building on read the docs goes, and we can do any amendments/fixes in another PR.

@khaledhosny khaledhosny merged commit 0998df3 into harfbuzz:main Nov 5, 2024
@khaledhosny
Copy link
Collaborator

Side note: if the pull request option is available for community plans then we may be able to avoid the duplicate runner effort of including a CI job for docs on GitHub too

I enabled it, I don’t know if it works or not.

@khaledhosny
Copy link
Collaborator

I did a few small tweaks and it seems the docs now build https://uharfbuzz.readthedocs.io/en/latest/, time to actually start documenting the code 😄

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.

Documentation
2 participants